@matheusbsbs / matheus-bagliano / commits / c2e8904258

git-gui: treat file names beginning with "|" as relative paths

The Tcl 'open' function has a very 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 "|", a process is spawned. We have a number of calls of Tcl 'open' that take a file name from the environment in which Git GUI is running. Be prepared that insane values are injected. In particular, when we intend to open a file, do not take a file name that happens to begin with "|" as a request to run a process. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>

Johannes Sixt committed Apr 21, 2025 at 17:07 UTC c2e8904258544f3d79dc4e96d1269c0ad8124db3
11 files changed +44 -32
git-gui.sh
+24 -12
@@ -170,6 +170,18 @@ proc open {args} {
170 uplevel 1 real_open $args
171 }
172
173 +# Wrap open to sanitize arguments
174 +
175 +proc safe_open_file {filename flags} {
176 + # a file name starting with "|" would attempt to run a process
177 + # but such a file name must be treated as a relative path
178 + # hide the "|" behind "./"
179 + if {[string index $filename 0] eq "|"} {
180 + set filename [file join . $filename]
181 + }
182 + open $filename $flags
183 +}
184 +
185 ######################################################################
186 ##
187 ## locate our library
@@ -494,7 +506,7 @@ proc _git_cmd {name} {
506 # Tcl on Windows doesn't know it.
507 #
508 set p [gitexec git-$name]
497 - set f [open $p r]
509 + set f [safe_open_file $p r]
510 set s [gets $f]
511 close $f
512
@@ -527,7 +539,7 @@ proc _git_cmd {name} {
539 # Test a file for a hashbang to identify executable scripts on Windows.
540 proc is_shellscript {filename} {
541 if {![file exists $filename]} {return 0}
530 - set f [open $filename r]
542 + set f [safe_open_file $filename r]
543 fconfigure $f -encoding binary
544 set magic [read $f 2]
545 close $f
@@ -683,7 +695,7 @@ proc sq {value} {
695 proc load_current_branch {} {
696 global current_branch is_detached
697
686 - set fd [open [gitdir HEAD] r]
698 + set fd [safe_open_file [gitdir HEAD] r]
699 fconfigure $fd -translation binary -encoding utf-8
700 if {[gets $fd ref] < 1} {
701 set ref {}
@@ -1045,7 +1057,7 @@ You are using [git-version]:
1057 ## configure our library
1058
1059 set idx [file join $oguilib tclIndex]
1048 -if {[catch {set fd [open $idx r]} err]} {
1060 +if {[catch {set fd [safe_open_file $idx r]} err]} {
1061 catch {wm withdraw .}
1062 tk_messageBox \
1063 -icon error \
@@ -1382,7 +1394,7 @@ proc repository_state {ctvar hdvar mhvar} {
1394 set merge_head [gitdir MERGE_HEAD]
1395 if {[file exists $merge_head]} {
1396 set ct merge
1385 - set fd_mh [open $merge_head r]
1397 + set fd_mh [safe_open_file $merge_head r]
1398 while {[gets $fd_mh line] >= 0} {
1399 lappend mh $line
1400 }
@@ -1530,7 +1542,7 @@ proc load_message {file {encoding {}}} {
1542
1543 set f [gitdir $file]
1544 if {[file isfile $f]} {
1533 - if {[catch {set fd [open $f r]}]} {
1545 + if {[catch {set fd [safe_open_file $f r]}]} {
1546 return 0
1547 }
1548 fconfigure $fd -eofchar {}
@@ -1554,23 +1566,23 @@ proc run_prepare_commit_msg_hook {} {
1566 # it will be .git/MERGE_MSG (merge), .git/SQUASH_MSG (squash), or an
1567 # empty file but existent file.
1568
1557 - set fd_pcm [open [gitdir PREPARE_COMMIT_MSG] a]
1569 + set fd_pcm [safe_open_file [gitdir PREPARE_COMMIT_MSG] a]
1570
1571 if {[file isfile [gitdir MERGE_MSG]]} {
1572 set pcm_source "merge"
1561 - set fd_mm [open [gitdir MERGE_MSG] r]
1573 + set fd_mm [safe_open_file [gitdir MERGE_MSG] r]
1574 fconfigure $fd_mm -encoding utf-8
1575 puts -nonewline $fd_pcm [read $fd_mm]
1576 close $fd_mm
1577 } elseif {[file isfile [gitdir SQUASH_MSG]]} {
1578 set pcm_source "squash"
1567 - set fd_sm [open [gitdir SQUASH_MSG] r]
1579 + set fd_sm [safe_open_file [gitdir SQUASH_MSG] r]
1580 fconfigure $fd_sm -encoding utf-8
1581 puts -nonewline $fd_pcm [read $fd_sm]
1582 close $fd_sm
1583 } elseif {[file isfile [get_config commit.template]]} {
1584 set pcm_source "template"
1573 - set fd_sm [open [get_config commit.template] r]
1585 + set fd_sm [safe_open_file [get_config commit.template] r]
1586 fconfigure $fd_sm -encoding utf-8
1587 puts -nonewline $fd_pcm [read $fd_sm]
1588 close $fd_sm
@@ -2271,7 +2283,7 @@ proc do_quit {{rc {1}}} {
2283 if {![string match amend* $commit_type]
2284 && $msg ne {}} {
2285 catch {
2274 - set fd [open $save w]
2286 + set fd [safe_open_file $save w]
2287 fconfigure $fd -encoding utf-8
2288 puts -nonewline $fd $msg
2289 close $fd
@@ -4032,7 +4044,7 @@ if {[winfo exists $ui_comm]} {
4044 }
4045 } elseif {$m} {
4046 catch {
4035 - set fd [open [gitdir GITGUI_BCK] w]
4047 + set fd [safe_open_file [gitdir GITGUI_BCK] w]
4048 fconfigure $fd -encoding utf-8
4049 puts -nonewline $fd $msg
4050 close $fd
lib/blame.tcl
+1 -1
@@ -481,7 +481,7 @@ method _load {jump} {
481 if {$do_textconv ne 0} {
482 set fd [open_cmd_pipe $textconv $path]
483 } else {
484 - set fd [open $path r]
484 + set fd [safe_open_file $path r]
485 }
486 fconfigure $fd -eofchar {}
487 } else {
lib/choose_repository.tcl
+7 -7
@@ -641,8 +641,8 @@ method _do_clone2 {} {
641 set pwd [pwd]
642 if {[catch {
643 file mkdir [gitdir objects info]
644 - set f_in [open [file join $objdir info alternates] r]
645 - set f_cp [open [gitdir objects info alternates] w]
644 + set f_in [safe_open_file [file join $objdir info alternates] r]
645 + set f_cp [safe_open_file [gitdir objects info alternates] w]
646 fconfigure $f_in -translation binary -encoding binary
647 fconfigure $f_cp -translation binary -encoding binary
648 cd $objdir
@@ -727,7 +727,7 @@ method _do_clone2 {} {
727 [cb _do_clone_tags]
728 }
729 shared {
730 - set fd [open [gitdir objects info alternates] w]
730 + set fd [safe_open_file [gitdir objects info alternates] w]
731 fconfigure $fd -translation binary
732 puts $fd $objdir
733 close $fd
@@ -760,8 +760,8 @@ method _copy_files {objdir tocopy} {
760 }
761 foreach p $tocopy {
762 if {[catch {
763 - set f_in [open [file join $objdir $p] r]
764 - set f_cp [open [file join .git objects $p] w]
763 + set f_in [safe_open_file [file join $objdir $p] r]
764 + set f_cp [safe_open_file [file join .git objects $p] w]
765 fconfigure $f_in -translation binary -encoding binary
766 fconfigure $f_cp -translation binary -encoding binary
767
@@ -823,7 +823,7 @@ method _clone_refs {} {
823 {--format=list %(refname) %(objectname) %(*objectname)}]
824 cd $pwd
825
826 - set fd [open [gitdir packed-refs] w]
826 + set fd [safe_open_file [gitdir packed-refs] w]
827 fconfigure $fd -translation binary
828 puts $fd "# pack-refs with: peeled"
829 while {[gets $fd_in line] >= 0} {
@@ -877,7 +877,7 @@ method _do_clone_full_end {ok} {
877
878 set HEAD {}
879 if {[file exists [gitdir FETCH_HEAD]]} {
880 - set fd [open [gitdir FETCH_HEAD] r]
880 + set fd [safe_open_file [gitdir FETCH_HEAD] r]
881 while {[gets $fd line] >= 0} {
882 if {[regexp "^(.{40})\t\t" $line line HEAD]} {
883 break
lib/choose_rev.tcl
+1 -1
@@ -579,7 +579,7 @@ method _reflog_last {name} {
579
580 set last {}
581 if {[catch {set last [file mtime [gitdir $name]]}]
582 - && ![catch {set g [open [gitdir logs $name] r]}]} {
582 + && ![catch {set g [safe_open_file [gitdir logs $name] r]}]} {
583 fconfigure $g -translation binary
584 while {[gets $g line] >= 0} {
585 if {[regexp {> ([1-9][0-9]*) } $line line when]} {
lib/commit.tcl
+2 -2
@@ -225,7 +225,7 @@ A good commit message has the following format:
225 # -- Build the message file.
226 #
227 set msg_p [gitdir GITGUI_EDITMSG]
228 - set msg_wt [open $msg_p w]
228 + set msg_wt [safe_open_file $msg_p w]
229 fconfigure $msg_wt -translation lf
230 setup_commit_encoding $msg_wt
231 puts $msg_wt $msg
@@ -409,7 +409,7 @@ A rescan will be automatically started now.
409 if {$commit_type ne {normal}} {
410 append reflogm " ($commit_type)"
411 }
412 - set msg_fd [open $msg_p r]
412 + set msg_fd [safe_open_file $msg_p r]
413 setup_commit_encoding $msg_fd 1
414 gets $msg_fd subject
415 close $msg_fd
lib/diff.tcl
+1 -1
@@ -202,7 +202,7 @@ proc show_other_diff {path w m cont_info} {
202 set sz [string length $content]
203 }
204 file {
205 - set fd [open $path r]
205 + set fd [safe_open_file $path r]
206 fconfigure $fd \
207 -eofchar {} \
208 -encoding [get_path_encoding $path]
lib/merge.tcl
+1 -1
@@ -93,7 +93,7 @@ method _start {} {
93 set spec [$w_rev get_tracking_branch]
94 set cmit [$w_rev get_commit]
95
96 - set fh [open [gitdir FETCH_HEAD] w]
96 + set fh [safe_open_file [gitdir FETCH_HEAD] w]
97 fconfigure $fh -translation lf
98 if {$spec eq {}} {
99 set remote .
lib/mergetool.tcl
+1 -1
@@ -293,7 +293,7 @@ proc merge_tool_get_stages {target stages} {
293 foreach fname $stages {
294 if {$merge_stages($i) eq {}} {
295 file delete $fname
296 - catch { close [open $fname w] }
296 + catch { close [safe_open_file $fname w] }
297 } else {
298 # A hack to support autocrlf properly
299 git checkout-index -f --stage=$i -- $target
lib/remote.tcl
+3 -3
@@ -75,7 +75,7 @@ proc load_all_remotes {} {
75
76 foreach name $all_remotes {
77 catch {
78 - set fd [open [file join $rm_dir $name] r]
78 + set fd [safe_open_file [file join $rm_dir $name] r]
79 while {[gets $fd line] >= 0} {
80 if {[regexp {^URL:[ ]*(.+)$} $line line url]} {
81 set remote_url($name) $url
@@ -145,7 +145,7 @@ proc add_fetch_entry {r} {
145 }
146 } else {
147 catch {
148 - set fd [open [gitdir remotes $r] r]
148 + set fd [safe_open_file [gitdir remotes $r] r]
149 while {[gets $fd n] >= 0} {
150 if {[regexp {^Pull:[ \t]*([^:]+):} $n]} {
151 set enable 1
@@ -182,7 +182,7 @@ proc add_push_entry {r} {
182 }
183 } else {
184 catch {
185 - set fd [open [gitdir remotes $r] r]
185 + set fd [safe_open_file [gitdir remotes $r] r]
186 while {[gets $fd n] >= 0} {
187 if {[regexp {^Push:[ \t]*([^:]+):} $n]} {
188 set enable 1
lib/shortcut.tcl
+2 -2
@@ -83,7 +83,7 @@ proc do_macosx_app {} {
83
84 file mkdir $MacOS
85
86 - set fd [open [file join $Contents Info.plist] w]
86 + set fd [safe_open_file [file join $Contents Info.plist] w]
87 puts $fd {<?xml version="1.0" encoding="UTF-8"?>
88 <!DOCTYPE plist PUBLIC "-//Apple Computer//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
89 <plist version="1.0">
@@ -108,7 +108,7 @@ proc do_macosx_app {} {
108 </plist>}
109 close $fd
110
111 - set fd [open $exe w]
111 + set fd [safe_open_file $exe w]
112 puts $fd "#!/bin/sh"
113 foreach name [lsort [array names env]] {
114 set value $env($name)
lib/sshkey.tcl
+1 -1
@@ -7,7 +7,7 @@ proc find_ssh_key {} {
7 ~/.ssh/id_rsa.pub ~/.ssh/identity.pub
8 } {
9 if {[file exists $name]} {
10 - set fh [open $name r]
10 + set fh [safe_open_file $name r]
11 set cont [read $fh]
12 close $fh
13 return [list $name $cont]