Skip to content

Commit b966b73

Browse files
j6tttaylorr
authored andcommitted
gitk: treat file names beginning with "|" as relative paths
The Tcl 'open' function has a vary wide interface. It can open files as well as pipes to external processes. The difference is made only by the first character of the file name: if it is "|", an process is spawned. We have a number of calls of Tcl 'open' that take a file name from the environment in which Gitk is running. Be prepared that insane values are injected. In particular, when we intend to open a file, do not mistake a file name that happens to begin with "|" as a request to run a process. Signed-off-by: Johannes Sixt <[email protected]> Signed-off-by: Taylor Blau <[email protected]>
1 parent 465f038 commit b966b73

File tree

1 file changed

+18
-5
lines changed

1 file changed

+18
-5
lines changed

gitk

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,19 @@ exec wish "$0" -- "$@"
99

1010
package require Tk
1111

12+
13+
# Wrap open to sanitize arguments
14+
15+
proc safe_open_file {filename flags} {
16+
# a file name starting with "|" would attempt to run a process
17+
# but such a file name must be treated as a relative path
18+
# hide the "|" behind "./"
19+
if {[string index $filename 0] eq "|"} {
20+
set filename [file join . $filename]
21+
}
22+
open $filename $flags
23+
}
24+
1225
proc hasworktree {} {
1326
return [expr {[exec git rev-parse --is-bare-repository] == "false" &&
1427
[exec git rev-parse --is-inside-git-dir] == "false"}]
@@ -2874,7 +2887,7 @@ proc savestuff {w} {
28742887
set remove_tmp 0
28752888
if {[catch {
28762889
set try_count 0
2877-
while {[catch {set f [open $config_file_tmp {WRONLY CREAT EXCL}]}]} {
2890+
while {[catch {set f [safe_open_file $config_file_tmp {WRONLY CREAT EXCL}]}]} {
28782891
if {[incr try_count] > 50} {
28792892
error "Unable to write config file: $config_file_tmp exists"
28802893
}
@@ -3869,7 +3882,7 @@ proc show_line_source {} {
38693882
# must be a merge in progress...
38703883
if {[catch {
38713884
# get the last line from .git/MERGE_HEAD
3872-
set f [open [file join $gitdir MERGE_HEAD] r]
3885+
set f [safe_open_file [file join $gitdir MERGE_HEAD] r]
38733886
set id [lindex [split [read $f] "\n"] end-1]
38743887
close $f
38753888
} err]} {
@@ -7723,7 +7736,7 @@ proc showfile {f} {
77237736
return
77247737
}
77257738
if {$diffids eq $nullid} {
7726-
if {[catch {set bf [open $f r]} err]} {
7739+
if {[catch {set bf [safe_open_file $f r]} err]} {
77277740
puts "oops, can't read $f: $err"
77287741
return
77297742
}
@@ -10200,7 +10213,7 @@ proc getallcommits {} {
1020010213
set cachedarcs 0
1020110214
set allccache [file join $gitdir "gitk.cache"]
1020210215
if {![catch {
10203-
set f [open $allccache r]
10216+
set f [safe_open_file $allccache r]
1020410217
set allcwait 1
1020510218
getcache $f
1020610219
}]} return
@@ -10624,7 +10637,7 @@ proc savecache {} {
1062410637
set cachearc 0
1062510638
set cachedarcs $nextarc
1062610639
catch {
10627-
set f [open $allccache w]
10640+
set f [safe_open_file $allccache w]
1062810641
puts $f [list 1 $cachedarcs]
1062910642
run writecache $f
1063010643
}

0 commit comments

Comments
 (0)