gitk: sanitize 'open' arguments: simple commands
Tcl 'open' treats the second argument as a command when it begins with |. The remainder of the argument is a list comprising the command and its arguments. It assigns special meaning to these arguments when they begin with a redirection, pipe or background operator. There are a number of invocations of 'open' 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 which have these special forms. They must not be interpreted by 'open' 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. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt committed
Mar 20, 2025 at 19:32 UTC
fe32bf31b8d5dff523543700ab76ecbf423a6d6f
1 file changed
+33
-26
gitk
+33
-26
@@ -59,6 +59,13 @@ proc safe_open_file {filename flags} {
59
open $filename $flags
60
}
61
62
+# opens a command pipeline for reading
63
+# cmd is a list that specifies the command and its arguments
64
+# calls `open` and returns the file id
65
+proc safe_open_command {cmd} {
66
+ open |[make_arglist_safe $cmd] r
67
+}
68
+
69
# End exec/open wrappers
70
71
proc hasworktree {} {
@@ -186,7 +193,7 @@ proc unmerged_files {files} {
193
set mlist {}
194
set nr_unmerged 0
195
if {[catch {
189
- set fd [open "| git ls-files -u" r]
196
+ set fd [safe_open_command {git ls-files -u}]
197
} err]} {
198
show_error {} . "[mc "Couldn't get list of unmerged files:"] $err"
199
exit 1
@@ -463,8 +470,8 @@ proc start_rev_list {view} {
470
}
471
472
if {[catch {
466
- set fd [open [concat | git log --no-color -z --pretty=raw $show_notes \
467
- --parents --boundary $args "--" $files] r]
473
+ set fd [safe_open_command [concat git log --no-color -z --pretty=raw $show_notes \
474
+ --parents --boundary $args "--" $files]]
475
} err]} {
476
error_popup "[mc "Error executing git log:"] $err"
477
return 0
@@ -611,8 +618,8 @@ proc updatecommits {} {
618
set args $vorigargs($view)
619
}
620
if {[catch {
614
- set fd [open [concat | git log --no-color -z --pretty=raw $show_notes \
615
- --parents --boundary $args "--" $vfilelimit($view)] r]
621
+ set fd [safe_open_command [concat git log --no-color -z --pretty=raw $show_notes \
622
+ --parents --boundary $args "--" $vfilelimit($view)]]
623
} err]} {
624
error_popup "[mc "Error executing git log:"] $err"
625
return
@@ -1700,7 +1707,7 @@ proc do_readcommit {id} {
1707
global tclencoding
1708
1709
# Invoke git-log to handle automatic encoding conversion
1703
- set fd [open [concat | git log --no-color --pretty=raw -1 $id] r]
1710
+ set fd [safe_open_command [concat git log --no-color --pretty=raw -1 $id]]
1711
# Read the results using i18n.logoutputencoding
1712
fconfigure $fd -translation lf -eofchar {}
1713
if {$tclencoding != {}} {
@@ -1836,7 +1843,7 @@ proc readrefs {} {
1843
foreach v {tagids idtags headids idheads otherrefids idotherrefs} {
1844
unset -nocomplain $v
1845
}
1839
- set refd [open [list | git show-ref -d] r]
1846
+ set refd [safe_open_command [list git show-ref -d]]
1847
if {$tclencoding != {}} {
1848
fconfigure $refd -encoding $tclencoding
1849
}
@@ -3729,7 +3736,7 @@ proc external_diff {} {
3736
3737
if {$difffromfile ne {} && $difftofile ne {}} {
3738
set cmd [list [shellsplit $extdifftool] $difffromfile $difftofile]
3732
- if {[catch {set fl [open |$cmd r]} err]} {
3739
+ if {[catch {set fl [safe_open_command $cmd]} err]} {
3740
file delete -force $diffdir
3741
error_popup "$extdifftool: [mc "command failed:"] $err"
3742
} else {
@@ -3833,7 +3840,7 @@ proc external_blame_diff {} {
3840
# Find the SHA1 ID of the blob for file $fname in the index
3841
# at stage 0 or 2
3842
proc index_sha1 {fname} {
3836
- set f [open [list | git ls-files -s $fname] r]
3843
+ set f [safe_open_command [list git ls-files -s $fname]]
3844
while {[gets $f line] >= 0} {
3845
set info [lindex [split $line "\t"] 0]
3846
set stage [lindex $info 2]
@@ -5311,8 +5318,8 @@ proc get_viewmainhead {view} {
5318
global viewmainheadid vfilelimit viewinstances mainheadid
5319
5320
catch {
5314
- set rfd [open [concat | git rev-list -1 $mainheadid \
5315
- -- $vfilelimit($view)] r]
5321
+ set rfd [safe_open_command [concat git rev-list -1 $mainheadid \
5322
+ -- $vfilelimit($view)]]
5323
set j [reg_instance $rfd]
5324
lappend viewinstances($view) $j
5325
fconfigure $rfd -blocking 0
@@ -5377,14 +5384,14 @@ proc dodiffindex {} {
5384
if {!$showlocalchanges || !$hasworktree} return
5385
incr lserial
5386
if {[package vcompare $git_version "1.7.2"] >= 0} {
5380
- set cmd "|git diff-index --cached --ignore-submodules=dirty HEAD"
5387
+ set cmd "git diff-index --cached --ignore-submodules=dirty HEAD"
5388
} else {
5382
- set cmd "|git diff-index --cached HEAD"
5389
+ set cmd "git diff-index --cached HEAD"
5390
}
5391
if {$vfilelimit($curview) ne {}} {
5392
set cmd [concat $cmd -- $vfilelimit($curview)]
5393
}
5387
- set fd [open $cmd r]
5394
+ set fd [safe_open_command $cmd]
5395
fconfigure $fd -blocking 0
5396
set i [reg_instance $fd]
5397
filerun $fd [list readdiffindex $fd $lserial $i]
@@ -5409,11 +5416,11 @@ proc readdiffindex {fd serial inst} {
5416
}
5417
5418
# now see if there are any local changes not checked in to the index
5412
- set cmd "|git diff-files"
5419
+ set cmd "git diff-files"
5420
if {$vfilelimit($curview) ne {}} {
5421
set cmd [concat $cmd -- $vfilelimit($curview)]
5422
}
5416
- set fd [open $cmd r]
5423
+ set fd [safe_open_command $cmd]
5424
fconfigure $fd -blocking 0
5425
set i [reg_instance $fd]
5426
filerun $fd [list readdifffiles $fd $serial $i]
@@ -7705,13 +7712,13 @@ proc gettree {id} {
7712
if {![info exists treefilelist($id)]} {
7713
if {![info exists treepending]} {
7714
if {$id eq $nullid} {
7708
- set cmd [list | git ls-files]
7715
+ set cmd [list git ls-files]
7716
} elseif {$id eq $nullid2} {
7710
- set cmd [list | git ls-files --stage -t]
7717
+ set cmd [list git ls-files --stage -t]
7718
} else {
7712
- set cmd [list | git ls-tree -r $id]
7719
+ set cmd [list git ls-tree -r $id]
7720
}
7714
- if {[catch {set gtf [open $cmd r]}]} {
7721
+ if {[catch {set gtf [safe_open_command $cmd]}]} {
7722
return
7723
}
7724
set treepending $id
@@ -7781,7 +7788,7 @@ proc showfile {f} {
7788
}
7789
} else {
7790
set blob [lindex $treeidlist($diffids) $i]
7784
- if {[catch {set bf [open [concat | git cat-file blob $blob] r]} err]} {
7791
+ if {[catch {set bf [safe_open_command [concat git cat-file blob $blob]]} err]} {
7792
puts "oops, error reading blob $blob: $err"
7793
return
7794
}
@@ -7976,7 +7983,7 @@ proc gettreediffs {ids} {
7983
if {$limitdiffs && $vfilelimit($curview) ne {}} {
7984
set cmd [concat $cmd -- $vfilelimit($curview)]
7985
}
7979
- if {[catch {set gdtf [open |$cmd r]}]} return
7986
+ if {[catch {set gdtf [safe_open_command $cmd]}]} return
7987
7988
set treepending $ids
7989
set treediff {}
@@ -8096,7 +8103,7 @@ proc getblobdiffs {ids} {
8103
if {$limitdiffs && $vfilelimit($curview) ne {}} {
8104
set cmd [concat $cmd -- $vfilelimit($curview)]
8105
}
8099
- if {[catch {set bdf [open |$cmd r]} err]} {
8106
+ if {[catch {set bdf [safe_open_command $cmd]} err]} {
8107
error_popup [mc "Error getting diffs: %s" $err]
8108
return
8109
}
@@ -9223,7 +9230,7 @@ proc diffcommits {a b} {
9230
return
9231
}
9232
if {[catch {
9226
- set fd [open "| diff -U$diffcontext $fna $fnb" r]
9233
+ set fd [safe_open_command "diff -U$diffcontext $fna $fnb"]
9234
} err]} {
9235
error_popup [mc "Error diffing commits: %s" $err]
9236
return
@@ -10256,7 +10263,7 @@ proc getallcommits {} {
10263
if {$allcwait} {
10264
return
10265
}
10259
- set cmd [list | git rev-list --parents]
10266
+ set cmd [list git rev-list --parents]
10267
set allcupdate [expr {$seeds ne {}}]
10268
if {!$allcupdate} {
10269
set ids "--all"
@@ -10281,7 +10288,7 @@ proc getallcommits {} {
10288
}
10289
}
10290
if {$ids ne {}} {
10284
- set fd [open [concat $cmd $ids] r]
10291
+ set fd [safe_open_command [concat $cmd $ids]]
10292
fconfigure $fd -blocking 0
10293
incr allcommits
10294
nowbusy allcommits