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>
Johannes Sixt committed
Apr 21, 2025 at 18:14 UTC
4f3e0a4bcef2c6caff68f96137d9914c5f2f98c2
4 files changed
+46
-15
git-gui.sh
+36
-6
@@ -170,7 +170,35 @@ proc open {args} {
170
uplevel 1 real_open $args
171
}
172
173
-# Wrap open to sanitize arguments
173
+# Wrap exec/open to sanitize arguments
174
+
175
+# unsafe arguments begin with redirections or the pipe or background operators
176
+proc is_arg_unsafe {arg} {
177
+ regexp {^([<|>&]|2>)} $arg
178
+}
179
+
180
+proc make_arg_safe {arg} {
181
+ if {[is_arg_unsafe $arg]} {
182
+ set arg [file join . $arg]
183
+ }
184
+ return $arg
185
+}
186
+
187
+proc make_arglist_safe {arglist} {
188
+ set res {}
189
+ foreach arg $arglist {
190
+ lappend res [make_arg_safe $arg]
191
+ }
192
+ return $res
193
+}
194
+
195
+# executes one command
196
+# no redirections or pipelines are possible
197
+# cmd is a list that specifies the command and its arguments
198
+# calls `exec` and returns its value
199
+proc safe_exec {cmd} {
200
+ eval exec [make_arglist_safe $cmd]
201
+}
202
203
proc safe_open_file {filename flags} {
204
# a file name starting with "|" would attempt to run a process
@@ -182,6 +210,8 @@ proc safe_open_file {filename flags} {
210
open $filename $flags
211
}
212
213
+# End exec/open wrappers
214
+
215
######################################################################
216
##
217
## locate our library
@@ -282,11 +312,11 @@ unset oguimsg
312
313
if {[tk windowingsystem] eq "aqua"} {
314
catch {
285
- exec osascript -e [format {
315
+ safe_exec [list osascript -e [format {
316
tell application "System Events"
317
set frontmost of processes whose unix id is %d to true
318
end tell
289
- } [pid]]
319
+ } [pid]]]
320
}
321
}
322
@@ -571,7 +601,7 @@ proc _lappend_nice {cmd_var} {
601
602
if {![info exists _nice]} {
603
set _nice [_which nice]
574
- if {[catch {exec $_nice git version}]} {
604
+ if {[catch {safe_exec [list $_nice git version]}]} {
605
set _nice {}
606
} elseif {[is_Windows] && [file dirname $_nice] ne [file dirname $::_git]} {
607
set _nice {}
@@ -667,9 +697,9 @@ proc kill_file_process {fd} {
697
698
catch {
699
if {[is_Windows]} {
670
- exec taskkill /pid $process
700
+ safe_exec [list taskkill /pid $process]
701
} else {
672
- exec kill $process
702
+ safe_exec [list kill $process]
703
}
704
}
705
}
lib/diff.tcl
+1
-1
@@ -226,7 +226,7 @@ proc show_other_diff {path w m cont_info} {
226
$ui_diff insert end \
227
"* [mc "Git Repository (subproject)"]\n" \
228
d_info
229
- } elseif {![catch {set type [exec file $path]}]} {
229
+ } elseif {![catch {set type [safe_exec [list file $path]]}]} {
230
set n [string length $path]
231
if {[string equal -length $n $path $type]} {
232
set type [string range $type $n end]
lib/shortcut.tcl
+4
-4
@@ -30,8 +30,8 @@ proc do_cygwin_shortcut {} {
30
global argv0 _gitworktree oguilib
31
32
if {[catch {
33
- set desktop [exec cygpath \
34
- --desktop]
33
+ set desktop [safe_exec [list cygpath \
34
+ --desktop]]
35
}]} {
36
set desktop .
37
}
@@ -50,14 +50,14 @@ proc do_cygwin_shortcut {} {
50
"CHERE_INVOKING=1 \
51
source /etc/profile; \
52
git gui"}
53
- exec /bin/mkshortcut.exe \
53
+ safe_exec [list /bin/mkshortcut.exe \
54
--arguments $shargs \
55
--desc "git-gui on $repodir" \
56
--icon $oguilib/git-gui.ico \
57
--name $fn \
58
--show min \
59
--workingdir $repodir \
60
- /bin/sh.exe
60
+ /bin/sh.exe]
61
} err]} {
62
error_popup [strcat [mc "Cannot write shortcut:"] "\n\n$err"]
63
}
lib/win32.tcl
+5
-4
@@ -2,11 +2,11 @@
2
# Copyright (C) 2007 Shawn Pearce
3
4
proc win32_read_lnk {lnk_path} {
5
- return [exec cscript.exe \
5
+ return [safe_exec [list cscript.exe \
6
/E:jscript \
7
/nologo \
8
[file join $::oguilib win32_shortcut.js] \
9
- $lnk_path]
9
+ $lnk_path]]
10
}
11
12
proc win32_create_lnk {lnk_path lnk_exec lnk_dir} {
@@ -15,12 +15,13 @@ proc win32_create_lnk {lnk_path lnk_exec lnk_dir} {
15
set lnk_args [lrange $lnk_exec 1 end]
16
set lnk_exec [lindex $lnk_exec 0]
17
18
- eval [list exec wscript.exe \
18
+ set cmd [list wscript.exe \
19
/E:jscript \
20
/nologo \
21
[file nativename [file join $oguilib win32_shortcut.js]] \
22
$lnk_path \
23
[file nativename [file join $oguilib git-gui.ico]] \
24
$lnk_dir \
25
- $lnk_exec] $lnk_args
25
+ $lnk_exec]
26
+ safe_exec [concat $cmd $lnk_args]
27
}