Skip to content

Commit 41fb138

Browse files
committed
Work around Tcl's default PATH lookup
As per https://www.tcl.tk/man/tcl8.6/TclCmd/exec.html#M23, Tcl's `exec` function goes out of its way to imitate the highly dangerous path lookup of `cmd.exe`, but _of course_ only on Windows: If a directory name was not specified as part of the application name, the following directories are automatically searched in order when attempting to locate the application: The directory from which the Tcl executable was loaded. The current directory. The Windows 32-bit system directory. The Windows home directory. The directories listed in the path. The dangerous part is the second item, of course: `exec` _prefers_ executables in the current directory to those that are actually in the `PATH`. It is almost as if people wanted to Windows users vulnerable, specifically. To avoid that, Git GUI already has the `_which` function that does not imitate that dangerous practice when looking up executables in the search path. However, Git GUI currently fails to use that function e.g. when trying to execute `aspell` for spell checking. That is not only dangerous but combined with Tcl's unfortunate default behavior and with the fact that Git GUI tries to spell-check a repository just after cloning, leads to a critical Remote Code Execution vulnerability. Let's override both `exec` and `open` to always use `_which` instead of letting Tcl perform the path lookup, to prevent this attack vector. This addresses CVE-2022-41953. For more details, see GHSA-v4px-mx59-w99c Signed-off-by: Johannes Schindelin <[email protected]>
1 parent bd4d74e commit 41fb138

File tree

1 file changed

+56
-0
lines changed

1 file changed

+56
-0
lines changed

git-gui/git-gui.sh

+56
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,62 @@ proc _which {what args} {
121121
return {}
122122
}
123123
124+
proc sanitize_command_line {command_line from_index} {
125+
set i $from_index
126+
while {$i < [llength $command_line]} {
127+
set cmd [lindex $command_line $i]
128+
if {[file pathtype $cmd] ne "absolute"} {
129+
set fullpath [_which $cmd]
130+
if {$fullpath eq ""} {
131+
throw {NOT-FOUND} "$cmd not found in PATH"
132+
}
133+
lset command_line $i $fullpath
134+
}
135+
136+
# handle piped commands, e.g. `exec A | B`
137+
for {incr i} {$i < [llength $command_line]} {incr i} {
138+
if {[lindex $command_line $i] eq "|"} {
139+
incr i
140+
break
141+
}
142+
}
143+
}
144+
return $command_line
145+
}
146+
147+
# Override `exec` to avoid unsafe PATH lookup
148+
149+
rename exec real_exec
150+
151+
proc exec {args} {
152+
# skip options
153+
for {set i 0} {$i < [llength $args]} {incr i} {
154+
set arg [lindex $args $i]
155+
if {$arg eq "--"} {
156+
incr i
157+
break
158+
}
159+
if {[string range $arg 0 0] ne "-"} {
160+
break
161+
}
162+
}
163+
set args [sanitize_command_line $args $i]
164+
uplevel 1 real_exec $args
165+
}
166+
167+
# Override `open` to avoid unsafe PATH lookup
168+
169+
rename open real_open
170+
171+
proc open {args} {
172+
set arg0 [lindex $args 0]
173+
if {[string range $arg0 0 0] eq "|"} {
174+
set command_line [string trim [string range $arg0 1 end]]
175+
lset args 0 "| [sanitize_command_line $command_line 0]"
176+
}
177+
uplevel 1 real_open $args
178+
}
179+
124180
######################################################################
125181
##
126182
## locate our library

0 commit comments

Comments
 (0)