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 https://github.com/git-for-windows/git/security/advisories/GHSA-v4px-mx59-w99c Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Pratyush Yadav <me@yadavpratyush.com>

Johannes Schindelin committed Nov 23, 2022 at 09:31 UTC aae9560a355d4ab91385e49eae62fade2ddd27ef
1 file changed +56
git-gui.sh
+56
@@ -121,6 +121,62 @@ proc _which {what args} {
121 return {}
122 }
123
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 +
180 ######################################################################
181 ##
182 ## locate our library