Merge branch 'ml/replace-auto-execok'

This addresses CVE-2025-46334, Git GUI malicious command injection on
Windows.

A malicious repository can ship versions of sh.exe or typical textconv
filter programs such as astextplain.  Due to the unfortunate design of
Tcl on Windows, the search path when looking for an executable always
includes the current directory.  The mentioned programs are invoked when
the user selects "Git Bash" or "Browse Files" from the menu.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>

git-gui/git-gui.sh

@@ -77,29 +77,49 @@ proc is_Cygwin {} {
 
 ######################################################################
 ##
-## PATH lookup
+## PATH lookup. Sanitize $PATH, assure exec/open use only that
 
-set _search_path {}
+if {[is_Windows]} {
-proc _which {what args} {
+	set _path_sep {;}
-	global env _search_exe _search_path
+	set _search_exe .exe
+} else {
+	set _path_sep {:}
+	set _search_exe {}
+}
 
-	if {$_search_path eq {}} {
+if {[is_Windows]} {
-		if {[is_Windows]} {
 	set gitguidir [file dirname [info script]]
 	regsub -all ";" $gitguidir "\\;" gitguidir
 	set env(PATH) "$gitguidir;$env(PATH)"
-			set _search_path [split $env(PATH) {;}]
+}
-			# Skip empty `PATH` elements
-			set _search_path [lsearch -all -inline -not -exact \
-				$_search_path ""]
-			set _search_exe .exe
-		} else {
-			set _search_path [split $env(PATH) :]
-			set _search_exe {}
-		}
-	}
 
-	if {[is_Windows] && [lsearch -exact $args -script] >= 0} {
+set _search_path {}
+set _path_seen [dict create]
+foreach p [split $env(PATH) $_path_sep] {
+	# Keep only absolute paths, getting rid of ., empty, etc.
+	if {[file pathtype $p] ne {absolute}} {
+		continue
+	}
+	# Keep only the first occurence of any duplicates.
+	set norm_p [file normalize $p]
+	if {[dict exists $_path_seen $norm_p]} {
+		continue
+	}
+	dict set _path_seen $norm_p 1
+	lappend _search_path $norm_p
+}
+unset _path_seen
+
+set env(PATH) [join $_search_path $_path_sep]
+
+if {[is_Windows]} {
+	proc _which {what args} {
+		global _search_exe _search_path
+
+		if {[lsearch -exact $args -script] >= 0} {
+			set suffix {}
+		} elseif {[string match *$_search_exe [string tolower $what]]} {
+			# The search string already has the file extension
 			set suffix {}
 		} else {
 			set suffix $_search_exe
@@ -112,9 +132,9 @@ proc _which {what args} {
 			}
 		}
 		return {}
-}
+	}
 
-proc sanitize_command_line {command_line from_index} {
+	proc sanitize_command_line {command_line from_index} {
 		set i $from_index
 		while {$i < [llength $command_line]} {
 			set cmd [lindex $command_line $i]
@@ -135,13 +155,13 @@ proc sanitize_command_line {command_line from_index} {
 			}
 		}
 		return $command_line
-}
+	}
 
-# Override `exec` to avoid unsafe PATH lookup
+	# Override `exec` to avoid unsafe PATH lookup
 
-rename exec real_exec
+	rename exec real_exec
 
-proc exec {args} {
+	proc exec {args} {
 		# skip options
 		for {set i 0} {$i < [llength $args]} {incr i} {
 			set arg [lindex $args $i]
@@ -155,19 +175,28 @@ proc exec {args} {
 		}
 		set args [sanitize_command_line $args $i]
 		uplevel 1 real_exec $args
-}
+	}
 
-# Override `open` to avoid unsafe PATH lookup
+	# Override `open` to avoid unsafe PATH lookup
 
-rename open real_open
+	rename open real_open
 
-proc open {args} {
+	proc open {args} {
 		set arg0 [lindex $args 0]
 		if {[string range $arg0 0 0] eq "|"} {
 			set command_line [string trim [string range $arg0 1 end]]
 			lset args 0 "| [sanitize_command_line $command_line 0]"
 		}
 		uplevel 1 real_open $args
+	}
+
+} else {
+	# On non-Windows platforms, auto_execok, exec, and open are safe, and will
+	# use the sanitized search path. But, we need _which for these.
+
+	proc _which {what args} {
+		return [lindex [auto_execok $what] 0]
+	}
 }
 
 ######################################################################
@@ -304,15 +333,37 @@ if {$_trace >= 0} {
 # branches).
 set _last_merged_branch {}
 
-proc shellpath {} {
+# for testing, allow unconfigured _shellpath
-	global _shellpath env
+if {[string match @@* $_shellpath]} {
-	if {[string match @@* $_shellpath]} {
 	if {[info exists env(SHELL)]} {
-			return $env(SHELL)
+		set _shellpath $env(SHELL)
 	} else {
-			return /bin/sh
+		set _shellpath /bin/sh
-		}
 	}
+}
+
+if {[is_Windows]} {
+	set _shellpath [exec cygpath -m $_shellpath]
+}
+
+if {![file executable $_shellpath] || \
+	!([file pathtype $_shellpath] eq {absolute})} {
+	set errmsg "The defined shell ('$_shellpath') is not usable, \
+		it must be an absolute path to an executable."
+	puts stderr $errmsg
+
+	catch {wm withdraw .}
+	tk_messageBox \
+		-icon error \
+		-type ok \
+		-title "git-gui: configuration error" \
+		-message $errmsg
+	exit 1
+}
+
+
+proc shellpath {} {
+	global _shellpath
 	return $_shellpath
 }
 
@@ -524,32 +575,13 @@ proc _git_cmd {name} {
 	return $v
 }
 
-# Test a file for a hashbang to identify executable scripts on Windows.
+# Run a shell command connected via pipes on stdout.
-proc is_shellscript {filename} {
-	if {![file exists $filename]} {return 0}
-	set f [open $filename r]
-	fconfigure $f -encoding binary
-	set magic [read $f 2]
-	close $f
-	return [expr {$magic eq "#!"}]
-}
-
-# Run a command connected via pipes on stdout.
 # This is for use with textconv filters and uses sh -c "..." to allow it to
-# contain a command with arguments. On windows we must check for shell
+# contain a command with arguments. We presume this
-# scripts specifically otherwise just call the filter command.
+# to be a shellscript that the configured shell (/bin/sh by default) knows
+# how to run.
 proc open_cmd_pipe {cmd path} {
-	global env
-	if {![file executable [shellpath]]} {
-		set exe [auto_execok [lindex $cmd 0]]
-		if {[is_shellscript [lindex $exe 0]]} {
-			set run [linsert [auto_execok sh] end -c "$cmd \"\$0\"" $path]
-		} else {
-			set run [concat $exe [lrange $cmd 1 end] $path]
-		}
-	} else {
 	set run [list [shellpath] -c "$cmd \"\$0\"" $path]
-	}
 	return [open |$run r]
 }
 
@@ -2753,17 +2785,16 @@ if {![is_bare]} {
 
 if {[is_Windows]} {
 	# Use /git-bash.exe if available
-	set normalized [file normalize $::argv0]
+	set _git_bash [exec cygpath -m /git-bash.exe]
-	regsub "/mingw../libexec/git-core/git-gui$" \
+	if {[file executable $_git_bash]} {
-		$normalized "/git-bash.exe" cmdLine
+		set _bash_cmdline [list "Git Bash" $_git_bash &]
-	if {$cmdLine != $normalized && [file exists $cmdLine]} {
-		set cmdLine [list "Git Bash" $cmdLine &]
 	} else {
-		set cmdLine [list "Git Bash" bash --login -l &]
+		set _bash_cmdline [list "Git Bash" bash --login -l &]
 	}
 	.mbar.repository add command \
 		-label [mc "Git Bash"] \
-		-command {eval exec [auto_execok start] $cmdLine}
+		-command {eval exec [list [_which cmd] /c start] $_bash_cmdline}
+	unset _git_bash
 }
 
 if {[is_Windows] || ![is_bare]} {

git-gui/lib/shortcut.tcl

@@ -12,7 +12,7 @@ proc do_windows_shortcut {} {
 			set fn ${fn}.lnk
 		}
 		# Use git-gui.exe if available (ie: git-for-windows)
-		set cmdLine [auto_execok git-gui.exe]
+		set cmdLine [list [_which git-gui]]
 		if {$cmdLine eq {}} {
 			set cmdLine [list [info nameofexecutable] \
 							 [file normalize $::argv0]]

git-gui/lib/sshkey.tcl

@@ -83,7 +83,8 @@ proc make_ssh_key {w} {
 	set sshkey_title [mc "Generating..."]
 	$w.header.gen configure -state disabled
 
-	set cmdline [list sh -c {echo | ssh-keygen -q -t rsa -f ~/.ssh/id_rsa 2>&1}]
+	set cmdline [list [shellpath] -c \
+		{echo | ssh-keygen -q -t rsa -f ~/.ssh/id_rsa 2>&1}]
 
 	if {[catch { set sshkey_fd [_open_stdout_stderr $cmdline] } err]} {
 		error_popup [mc "Could not start ssh-keygen:\n\n%s" $err]

git-gui/lib/tools.tcl

@@ -110,14 +110,14 @@ proc tools_exec {fullname} {
 
 	set cmdline $repo_config(guitool.$fullname.cmd)
 	if {[is_config_true "guitool.$fullname.noconsole"]} {
-		tools_run_silent [list sh -c $cmdline] \
+		tools_run_silent [list [shellpath] -c $cmdline] \
 				 [list tools_complete $fullname {}]
 	} else {
 		regsub {/} $fullname { / } title
 		set w [console::new \
 			[mc "Tool: %s" $title] \
 			[mc "Running: %s" $cmdline]]
-		console::exec $w [list sh -c $cmdline] \
+		console::exec $w [list [shellpath] -c $cmdline] \
 				 [list tools_complete $fullname $w]
 	}