git-gui: sanitize 'exec' arguments: simple cases

Tcl 'exec' assigns special meaning to its argument when they begin with
redirection, pipe or background operator. There are a number of
invocations of 'exec' which construct arguments that are taken from the
Git repository or a user input. However, when file names or ref names
are taken from the repository, it is possible to find names that have
these special forms. They must not be interpreted by 'exec' lest it
redirects input or output, or attempts to build a pipeline using a
command name controlled by the repository.

Introduce a helper function that identifies such arguments and prepends
"./" to force such a name to be regarded as a relative file name.

Convert those 'exec' calls where the arguments can simply be packed
into a list.

Note that most commands containing the word 'exec' route through
console::exec or console::chain, which we will treat in another commit.

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

Signed-off-by: Taylor Blau <me@ttaylorr.com>

4 files changed, 46 insertions, 15 deletions

diff --git a/git-gui.sh b/git-gui.sh
index 52b463c45f..a6be30e295 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -170,7 +170,35 @@ proc open {args} {
 	uplevel 1 real_open $args
 }
 
-# Wrap open to sanitize arguments
+# Wrap exec/open to sanitize arguments
+
+# unsafe arguments begin with redirections or the pipe or background operators
+proc is_arg_unsafe {arg} {
+	regexp {^([<|>&]|2>)} $arg
+}
+
+proc make_arg_safe {arg} {
+	if {[is_arg_unsafe $arg]} {
+		set arg [file join . $arg]
+	}
+	return $arg
+}
+
+proc make_arglist_safe {arglist} {
+	set res {}
+	foreach arg $arglist {
+		lappend res [make_arg_safe $arg]
+	}
+	return $res
+}
+
+# executes one command
+# no redirections or pipelines are possible
+# cmd is a list that specifies the command and its arguments
+# calls `exec` and returns its value
+proc safe_exec {cmd} {
+	eval exec [make_arglist_safe $cmd]
+}
 
 proc safe_open_file {filename flags} {
 	# a file name starting with "|" would attempt to run a process
@@ -182,6 +210,8 @@ proc safe_open_file {filename flags} {
 	open $filename $flags
 }
 
+# End exec/open wrappers
+
 ######################################################################
 ##
 ## locate our library
@@ -282,11 +312,11 @@ unset oguimsg
 
 if {[tk windowingsystem] eq "aqua"} {
 	catch {
-		exec osascript -e [format {
+		safe_exec [list osascript -e [format {
 			tell application "System Events"
 				set frontmost of processes whose unix id is %d to true
 			end tell
-		} [pid]]
+		} [pid]]]
 	}
 }
 
@@ -571,7 +601,7 @@ proc _lappend_nice {cmd_var} {
 
 	if {![info exists _nice]} {
 		set _nice [_which nice]
-		if {[catch {exec $_nice git version}]} {
+		if {[catch {safe_exec [list $_nice git version]}]} {
 			set _nice {}
 		} elseif {[is_Windows] && [file dirname $_nice] ne [file dirname $::_git]} {
 			set _nice {}
@@ -667,9 +697,9 @@ proc kill_file_process {fd} {
 
 	catch {
 		if {[is_Windows]} {
-			exec taskkill /pid $process
+			safe_exec [list taskkill /pid $process]
 		} else {
-			exec kill $process
+			safe_exec [list kill $process]
 		}
 	}
 }
diff --git a/lib/diff.tcl b/lib/diff.tcl
index f089fdc46b..cfa8a414d3 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -226,7 +226,7 @@ proc show_other_diff {path w m cont_info} {
 			$ui_diff insert end \
 				"* [mc "Git Repository (subproject)"]\n" \
 				d_info
-		} elseif {![catch {set type [exec file $path]}]} {
+		} elseif {![catch {set type [safe_exec [list file $path]]}]} {
 			set n [string length $path]
 			if {[string equal -length $n $path $type]} {
 				set type [string range $type $n end]
diff --git a/lib/shortcut.tcl b/lib/shortcut.tcl
index 9be79b2e89..d437ea6933 100644
--- a/lib/shortcut.tcl
+++ b/lib/shortcut.tcl
@@ -30,8 +30,8 @@ proc do_cygwin_shortcut {} {
 	global argv0 _gitworktree oguilib
 
 	if {[catch {
-		set desktop [exec cygpath \
-			--desktop]
+		set desktop [safe_exec [list cygpath \
+			--desktop]]
 		}]} {
 			set desktop .
 	}
@@ -50,14 +50,14 @@ proc do_cygwin_shortcut {} {
 					"CHERE_INVOKING=1 \
 					source /etc/profile; \
 					git gui"}
-				exec /bin/mkshortcut.exe \
+				safe_exec [list /bin/mkshortcut.exe \
 					--arguments $shargs \
 					--desc "git-gui on $repodir" \
 					--icon $oguilib/git-gui.ico \
 					--name $fn \
 					--show min \
 					--workingdir $repodir \
-					/bin/sh.exe
+					/bin/sh.exe]
 			} err]} {
 			error_popup [strcat [mc "Cannot write shortcut:"] "\n\n$err"]
 		}
diff --git a/lib/win32.tcl b/lib/win32.tcl
index db91ab84a5..3aedae2f13 100644
--- a/lib/win32.tcl
+++ b/lib/win32.tcl
@@ -2,11 +2,11 @@
 # Copyright (C) 2007 Shawn Pearce
 
 proc win32_read_lnk {lnk_path} {
-	return [exec cscript.exe \
+	return [safe_exec [list cscript.exe \
 		/E:jscript \
 		/nologo \
 		[file join $::oguilib win32_shortcut.js] \
-		$lnk_path]
+		$lnk_path]]
 }
 
 proc win32_create_lnk {lnk_path lnk_exec lnk_dir} {
@@ -15,12 +15,13 @@ proc win32_create_lnk {lnk_path lnk_exec lnk_dir} {
 	set lnk_args [lrange $lnk_exec 1 end]
 	set lnk_exec [lindex $lnk_exec 0]
 
-	eval [list exec wscript.exe \
+	set cmd [list wscript.exe \
 		/E:jscript \
 		/nologo \
 		[file nativename [file join $oguilib win32_shortcut.js]] \
 		$lnk_path \
 		[file nativename [file join $oguilib git-gui.ico]] \
 		$lnk_dir \
-		$lnk_exec] $lnk_args
+		$lnk_exec]
+	safe_exec [concat $cmd $lnk_args]
 }