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>

diff --git a/git-gui.sh b/git-gui.sh
index 52b463c45fb582be7fceb3ae1ceabd589fa867e7..a6be30e295bb41339eb1807eb4943f1abb9d9df2 100755 (executable)
--- 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 f089fdc46b21973168524f88bc1e93f70ccea92c..cfa8a414d30d1647086725ec300e04c794897dbc 100644 (file)
--- 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 9be79b2e895ff6cbcd555b15fea82b78c7387b60..d437ea69339dc61fc7c799454ba096330016f81e 100644 (file)
--- 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 db91ab84a56d79be6a5497f885a9181f368b9cf2..3aedae2f135dbc18ae9c5b83703d42ba175b0f74 100644 (file)
--- 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]
 }