diff --git a/backport-CVE-2025-27613-CVE-2025-46334-CVE-2025-46835.patch b/backport-CVE-2025-27613-CVE-2025-46334-CVE-2025-46835.patch new file mode 100644 index 0000000000000000000000000000000000000000..a2e0203f96e3296862b22e804564f4c6931a9ada --- /dev/null +++ b/backport-CVE-2025-27613-CVE-2025-46334-CVE-2025-46835.patch @@ -0,0 +1,3870 @@ +From b966b738e1923badc788b9111cc81653b50ff164 Mon Sep 17 00:00:00 2001 +From: Johannes Sixt +Date: Mon, 17 Mar 2025 20:36:04 +0100 +Subject: [PATCH 01/41] 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 +Signed-off-by: Taylor Blau +--- + gitk | 23 ++++++++++++++++++----- + 1 file changed, 18 insertions(+), 5 deletions(-) + +diff --git a/gitk-git/gitk b/gitk-git/gitk +index 0ae7d685904b85..ffb5382b494d68 100755 +--- a/gitk-git/gitk ++++ b/gitk-git/gitk +@@ -9,6 +9,19 @@ exec wish "$0" -- "$@" + + package require Tk + ++ ++# Wrap open to sanitize arguments ++ ++proc safe_open_file {filename flags} { ++ # a file name starting with "|" would attempt to run a process ++ # but such a file name must be treated as a relative path ++ # hide the "|" behind "./" ++ if {[string index $filename 0] eq "|"} { ++ set filename [file join . $filename] ++ } ++ open $filename $flags ++} ++ + proc hasworktree {} { + return [expr {[exec git rev-parse --is-bare-repository] == "false" && + [exec git rev-parse --is-inside-git-dir] == "false"}] +@@ -2874,7 +2887,7 @@ proc savestuff {w} { + set remove_tmp 0 + if {[catch { + set try_count 0 +- while {[catch {set f [open $config_file_tmp {WRONLY CREAT EXCL}]}]} { ++ while {[catch {set f [safe_open_file $config_file_tmp {WRONLY CREAT EXCL}]}]} { + if {[incr try_count] > 50} { + error "Unable to write config file: $config_file_tmp exists" + } +@@ -3869,7 +3882,7 @@ proc show_line_source {} { + # must be a merge in progress... + if {[catch { + # get the last line from .git/MERGE_HEAD +- set f [open [file join $gitdir MERGE_HEAD] r] ++ set f [safe_open_file [file join $gitdir MERGE_HEAD] r] + set id [lindex [split [read $f] "\n"] end-1] + close $f + } err]} { +@@ -7723,7 +7736,7 @@ proc showfile {f} { + return + } + if {$diffids eq $nullid} { +- if {[catch {set bf [open $f r]} err]} { ++ if {[catch {set bf [safe_open_file $f r]} err]} { + puts "oops, can't read $f: $err" + return + } +@@ -10200,7 +10213,7 @@ proc getallcommits {} { + set cachedarcs 0 + set allccache [file join $gitdir "gitk.cache"] + if {![catch { +- set f [open $allccache r] ++ set f [safe_open_file $allccache r] + set allcwait 1 + getcache $f + }]} return +@@ -10624,7 +10637,7 @@ proc savecache {} { + set cachearc 0 + set cachedarcs $nextarc + catch { +- set f [open $allccache w] ++ set f [safe_open_file $allccache w] + puts $f [list 1 $cachedarcs] + run writecache $f + } + +From 6eb797f5d1d8885c0f08e42cc5291c11be6f11a4 Mon Sep 17 00:00:00 2001 +From: Johannes Sixt +Date: Mon, 17 Mar 2025 21:39:58 +0100 +Subject: [PATCH 02/41] gitk: have callers of diffcmd supply pipe symbol when + necessary + +Function 'diffcmd' derives which of git diff-files, git diff-index, or +git diff-tree must be invoked depending on the ids provided. It puts +the pipe symbol as the first element of the returned command list. + +Note though that of the four callers only two use the command with +Tcl 'open' and need the pipe symbol. The other two callers pass the +command to Tcl 'exec' and must remove the pipe symbol. + +Do not include the pipe symbol in the constructed command list, but let +the call sites decide whether to add it or not. Note that Tcl 'open' +inspects only the first character of the command list, which is also +the first character of the first element in the list. For this reason, +it is valid to just tack on the pipe symbol with |$cmd and it is not +necessary to use [concat | $cmd]. + +Signed-off-by: Johannes Sixt + +Signed-off-by: Taylor Blau +--- + gitk | 16 ++++++---------- + 1 file changed, 6 insertions(+), 10 deletions(-) + +diff --git a/gitk-git/gitk b/gitk-git/gitk +index ffb5382b494d68..b061377c7bc79c 100755 +--- a/gitk-git/gitk ++++ b/gitk-git/gitk +@@ -7892,7 +7892,7 @@ proc diffcmd {ids flags} { + if {$i >= 0} { + if {[llength $ids] > 1 && $j < 0} { + # comparing working directory with some specific revision +- set cmd [concat | git diff-index $flags] ++ set cmd [concat git diff-index $flags] + if {$i == 0} { + lappend cmd -R [lindex $ids 1] + } else { +@@ -7900,7 +7900,7 @@ proc diffcmd {ids flags} { + } + } else { + # comparing working directory with index +- set cmd [concat | git diff-files $flags] ++ set cmd [concat git diff-files $flags] + if {$j == 1} { + lappend cmd -R + } +@@ -7909,7 +7909,7 @@ proc diffcmd {ids flags} { + if {[package vcompare $git_version "1.7.2"] >= 0} { + set flags "$flags --ignore-submodules=dirty" + } +- set cmd [concat | git diff-index --cached $flags] ++ set cmd [concat git diff-index --cached $flags] + if {[llength $ids] > 1} { + # comparing index with specific revision + if {$j == 0} { +@@ -7925,7 +7925,7 @@ proc diffcmd {ids flags} { + if {$log_showroot} { + lappend flags --root + } +- set cmd [concat | git diff-tree -r $flags $ids] ++ set cmd [concat git diff-tree -r $flags $ids] + } + return $cmd + } +@@ -7937,7 +7937,7 @@ proc gettreediffs {ids} { + if {$limitdiffs && $vfilelimit($curview) ne {}} { + set cmd [concat $cmd -- $vfilelimit($curview)] + } +- if {[catch {set gdtf [open $cmd r]}]} return ++ if {[catch {set gdtf [open |$cmd r]}]} return + + set treepending $ids + set treediff {} +@@ -8057,7 +8057,7 @@ proc getblobdiffs {ids} { + if {$limitdiffs && $vfilelimit($curview) ne {}} { + set cmd [concat $cmd -- $vfilelimit($curview)] + } +- if {[catch {set bdf [open $cmd r]} err]} { ++ if {[catch {set bdf [open |$cmd r]} err]} { + error_popup [mc "Error getting diffs: %s" $err] + return + } +@@ -9080,8 +9080,6 @@ proc getpatchid {id} { + + if {![info exists patchids($id)]} { + set cmd [diffcmd [list $id] {-p --root}] +- # trim off the initial "|" +- set cmd [lrange $cmd 1 end] + if {[catch { + set x [eval exec $cmd | git patch-id] + set patchids($id) [lindex $x 0] +@@ -9326,8 +9324,6 @@ proc mkpatchgo {} { + set newid [$patchtop.tosha1 get] + set fname [$patchtop.fname get] + set cmd [diffcmd [list $oldid $newid] -p] +- # trim off the initial "|" +- set cmd [lrange $cmd 1 end] + lappend cmd >$fname & + if {[catch {eval exec $cmd} err]} { + error_popup "[mc "Error creating patch:"] $err" $patchtop + +From 9f0d1c2f7d2543cd2549cbc26e142fd199f18b45 Mon Sep 17 00:00:00 2001 +From: Johannes Sixt +Date: Mon, 17 Mar 2025 22:59:27 +0100 +Subject: [PATCH 03/41] gitk: 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 with 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. + +Signed-off-by: Johannes Sixt +Signed-off-by: Taylor Blau +--- + gitk | 66 +++++++++++++++++++++++++++++++++++++++++++----------------- + 1 file changed, 48 insertions(+), 18 deletions(-) + +diff --git a/gitk-git/gitk b/gitk-git/gitk +index b061377c7bc79c..5b844730b1e4a6 100755 +--- a/gitk-git/gitk ++++ b/gitk-git/gitk +@@ -10,7 +10,35 @@ exec wish "$0" -- "$@" + package require Tk + + +-# 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 +@@ -22,6 +50,8 @@ proc safe_open_file {filename flags} { + open $filename $flags + } + ++# End exec/open wrappers ++ + proc hasworktree {} { + return [expr {[exec git rev-parse --is-bare-repository] == "false" && + [exec git rev-parse --is-inside-git-dir] == "false"}] +@@ -387,7 +417,7 @@ proc start_rev_list {view} { + set args $viewargs($view) + if {$viewargscmd($view) ne {}} { + if {[catch { +- set str [exec sh -c $viewargscmd($view)] ++ set str [safe_exec [list sh -c $viewargscmd($view)]] + } err]} { + error_popup "[mc "Error executing --argscmd command:"] $err" + return 0 +@@ -459,9 +489,9 @@ proc stop_instance {inst} { + set pid [pid $fd] + + if {$::tcl_platform(platform) eq {windows}} { +- exec taskkill /pid $pid ++ safe_exec [list taskkill /pid $pid] + } else { +- exec kill $pid ++ safe_exec [list kill $pid] + } + } + catch {close $fd} +@@ -1540,8 +1570,8 @@ proc getcommitlines {fd inst view updating} { + # and if we already know about it, using the rewritten + # parent as a substitute parent for $id's children. + if {![catch { +- set rwid [exec git rev-list --first-parent --max-count=1 \ +- $id -- $vfilelimit($view)] ++ set rwid [safe_exec [list git rev-list --first-parent --max-count=1 \ ++ $id -- $vfilelimit($view)]] + }]} { + if {$rwid ne {} && [info exists varcid($view,$rwid)]} { + # use $rwid in place of $id +@@ -1845,7 +1875,7 @@ proc readrefs {} { + set selectheadid {} + if {$selecthead ne {}} { + catch { +- set selectheadid [exec git rev-parse --verify $selecthead] ++ set selectheadid [safe_exec [list git rev-parse --verify $selecthead]] + } + } + } +@@ -3603,7 +3633,7 @@ proc gitknewtmpdir {} { + set tmpdir $gitdir + } + set gitktmpformat [file join $tmpdir ".gitk-tmp.XXXXXX"] +- if {[catch {set gitktmpdir [exec mktemp -d $gitktmpformat]}]} { ++ if {[catch {set gitktmpdir [safe_exec [list mktemp -d $gitktmpformat]]}]} { + set gitktmpdir [file join $gitdir [format ".gitk-tmp.%s" [pid]]] + } + if {[catch {file mkdir $gitktmpdir} err]} { +@@ -8774,7 +8804,7 @@ proc gotocommit {} { + set id [lindex $matches 0] + } + } else { +- if {[catch {set id [exec git rev-parse --verify $sha1string]}]} { ++ if {[catch {set id [safe_exec [list git rev-parse --verify $sha1string]]}]} { + error_popup [mc "Revision %s is not known" $sha1string] + return + } +@@ -9394,9 +9424,9 @@ proc domktag {} { + } + if {[catch { + if {$msg != {}} { +- exec git tag -a -m $msg $tag $id ++ safe_exec [list git tag -a -m $msg $tag $id] + } else { +- exec git tag $tag $id ++ safe_exec [list git tag $tag $id] + } + } err]} { + error_popup "[mc "Error creating tag:"] $err" $mktagtop +@@ -9723,7 +9753,7 @@ proc cherrypick {} { + update + # Unfortunately git-cherry-pick writes stuff to stderr even when + # no error occurs, and exec takes that as an indication of error... +- if {[catch {exec sh -c "git cherry-pick -r $rowmenuid 2>&1"} err]} { ++ if {[catch {safe_exec [list sh -c "git cherry-pick -r $rowmenuid 2>&1"]} err]} { + notbusy cherrypick + if {[regexp -line \ + {Entry '(.*)' (would be overwritten by merge|not uptodate)} \ +@@ -9785,7 +9815,7 @@ proc revert {} { + nowbusy revert [mc "Reverting"] + update + +- if [catch {exec git revert --no-edit $rowmenuid} err] { ++ if [catch {safe_exec [list git revert --no-edit $rowmenuid]} err] { + notbusy revert + if [regexp {files would be overwritten by merge:(\n(( |\t)+[^\n]+\n)+)}\ + $err match files] { +@@ -10018,7 +10048,7 @@ proc rmbranch {} { + } + nowbusy rmbranch + update +- if {[catch {exec git branch -D $head} err]} { ++ if {[catch {safe_exec [list git branch -D $head]} err]} { + notbusy rmbranch + error_popup $err + return +@@ -11336,7 +11366,7 @@ proc add_tag_ctext {tag} { + + if {![info exists cached_tagcontent($tag)]} { + catch { +- set cached_tagcontent($tag) [exec git cat-file -p $tag] ++ set cached_tagcontent($tag) [safe_exec [list git cat-file -p $tag]] + } + } + $ctext insert end "[mc "Tag"]: $tag\n" bold +@@ -12222,7 +12252,7 @@ proc gitattr {path attr default} { + set r $path_attr_cache($attr,$path) + } else { + set r "unspecified" +- if {![catch {set line [exec git check-attr $attr -- $path]}]} { ++ if {![catch {set line [safe_exec [list git check-attr $attr -- $path]]}]} { + regexp "(.*): $attr: (.*)" $line m f r + } + set path_attr_cache($attr,$path) $r +@@ -12302,11 +12332,11 @@ if {[catch {package require Tk 8.4} err]} { + + # on OSX bring the current Wish process window to front + if {[tk windowingsystem] eq "aqua"} { +- 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] ]] + } + + # Unset GIT_TRACE var if set + +From 88139a617f3fe768ff8d026031811855906b69bc Mon Sep 17 00:00:00 2001 +From: Johannes Sixt +Date: Sat, 29 Mar 2025 16:51:29 +0100 +Subject: [PATCH 04/41] gitk: sanitize 'exec' arguments: 'eval exec' + +Convert calls of 'exec' where the arguments are already available in +a list and 'eval' is used to unpack the list. Use 'concat' to unite +the arguments into a single list before passing them to 'safe_exec'. + +Signed-off-by: Johannes Sixt +Signed-off-by: Taylor Blau +--- + gitk | 12 ++++++------ + 1 file changed, 6 insertions(+), 6 deletions(-) + +diff --git a/gitk-git/gitk b/gitk-git/gitk +index 5b844730b1e4a6..d748d19d9091c1 100755 +--- a/gitk-git/gitk ++++ b/gitk-git/gitk +@@ -339,7 +339,7 @@ proc parseviewrevs {view revs} { + } elseif {[lsearch -exact $revs --all] >= 0} { + lappend revs HEAD + } +- if {[catch {set ids [eval exec git rev-parse $revs]} err]} { ++ if {[catch {set ids [safe_exec [concat git rev-parse $revs]]} err]} { + # we get stdout followed by stderr in $err + # for an unknown rev, git rev-parse echoes it and then errors out + set errlines [split $err "\n"] +@@ -9494,7 +9494,7 @@ proc copyreference {} { + if {$autosellen < 40} { + lappend cmd --abbrev=$autosellen + } +- set reference [eval exec $cmd $rowmenuid] ++ set reference [safe_exec [concat $cmd $rowmenuid]] + + clipboard clear + clipboard append $reference +@@ -9648,7 +9648,7 @@ proc mkbrgo {top} { + nowbusy newbranch + update + if {[catch { +- eval exec git branch $cmdargs ++ safe_exec [concat git branch $cmdargs] + } err]} { + notbusy newbranch + error_popup $err +@@ -9689,7 +9689,7 @@ proc mvbrgo {top prevname} { + nowbusy renamebranch + update + if {[catch { +- eval exec git branch $cmdargs ++ safe_exec [concat git branch $cmdargs] + } err]} { + notbusy renamebranch + error_popup $err +@@ -12279,7 +12279,7 @@ proc cache_gitattr {attr pathlist} { + while {$newlist ne {}} { + set head [lrange $newlist 0 [expr {$lim - 1}]] + set newlist [lrange $newlist $lim end] +- if {![catch {set rlist [eval exec git check-attr $attr -- $head]}]} { ++ if {![catch {set rlist [safe_exec [concat git check-attr $attr -- $head]]}]} { + foreach row [split $rlist "\n"] { + if {[regexp "(.*): $attr: (.*)" $row m path value]} { + if {[string index $path 0] eq "\""} { +@@ -12581,7 +12581,7 @@ if {$selecthead eq "HEAD"} { + if {$i >= [llength $argv] && $revtreeargs ne {}} { + # no -- on command line, but some arguments (other than --argscmd) + if {[catch { +- set f [eval exec git rev-parse --no-revs --no-flags $revtreeargs] ++ set f [safe_exec [concat git rev-parse --no-revs --no-flags $revtreeargs]] + set cmdline_files [split $f "\n"] + set n [llength $cmdline_files] + set revtreeargs [lrange $revtreeargs 0 end-$n] + +From 6b631ee8ed76c13a28e482588fd1264d591a46de Mon Sep 17 00:00:00 2001 +From: Johannes Sixt +Date: Sat, 29 Mar 2025 17:01:54 +0100 +Subject: [PATCH 05/41] gitk: sanitize 'exec' arguments: redirections + +As in the previous commits, introduce a function that sanitizes +arguments intended for the process and in addition allows to pass +redirections verbatim, which are interpreted by Tcl's 'exec'. +Redirections can include the background operator '&'. + +Signed-off-by: Johannes Sixt +Signed-off-by: Taylor Blau +--- + gitk | 25 +++++++++++++++++-------- + 1 file changed, 17 insertions(+), 8 deletions(-) + +diff --git a/gitk-git/gitk b/gitk-git/gitk +index d748d19d9091c1..218f61fa28f223 100755 +--- a/gitk-git/gitk ++++ b/gitk-git/gitk +@@ -40,6 +40,15 @@ proc safe_exec {cmd} { + eval exec [make_arglist_safe $cmd] + } + ++# executes one command with redirections ++# no pipelines are possible ++# cmd is a list that specifies the command and its arguments ++# redir is a list that specifies redirections (output, background) ++# calls `exec` and returns its value ++proc safe_exec_redirect {cmd redir} { ++ eval exec [make_arglist_safe $cmd] $redir ++} ++ + proc safe_open_file {filename flags} { + # a file name starting with "|" would attempt to run a process + # but such a file name must be treated as a relative path +@@ -2135,7 +2144,7 @@ proc makewindow {} { + {mc "Reread re&ferences" command rereadrefs} + {mc "&List references" command showrefs -accelerator F2} + {xx "" separator} +- {mc "Start git &gui" command {exec git gui &}} ++ {mc "Start git &gui" command {safe_exec_redirect [list git gui] [list &]}} + {xx "" separator} + {mc "&Quit" command doquit -accelerator Meta1-Q} + }} +@@ -3655,7 +3664,7 @@ proc gitknewtmpdir {} { + proc save_file_from_commit {filename output what} { + global nullfile + +- if {[catch {exec git show $filename -- > $output} err]} { ++ if {[catch {safe_exec_redirect [list git show $filename --] [list > $output]} err]} { + if {[string match "fatal: bad revision *" $err]} { + return $nullfile + } +@@ -3884,7 +3893,7 @@ proc external_blame {parent_idx {line {}}} { + # being given an absolute path... + set f [make_relative $f] + lappend cmdline $base_commit $f +- if {[catch {eval exec $cmdline &} err]} { ++ if {[catch {safe_exec_redirect $cmdline [list &]} err]} { + error_popup "[mc "git gui blame: command failed:"] $err" + } + } +@@ -7193,8 +7202,8 @@ proc browseweb {url} { + global web_browser + + if {$web_browser eq {}} return +- # Use eval here in case $web_browser is a command plus some arguments +- if {[catch {eval exec $web_browser [list $url] &} err]} { ++ # Use concat here in case $web_browser is a command plus some arguments ++ if {[catch {safe_exec_redirect [concat $web_browser [list $url]] [list &]} err]} { + error_popup "[mc "Error starting web browser:"] $err" + } + } +@@ -9207,8 +9216,8 @@ proc diffcommits {a b} { + set fna [file join $tmpdir "commit-[string range $a 0 7]"] + set fnb [file join $tmpdir "commit-[string range $b 0 7]"] + if {[catch { +- exec git diff-tree -p --pretty $a >$fna +- exec git diff-tree -p --pretty $b >$fnb ++ safe_exec_redirect [list git diff-tree -p --pretty $a] [list >$fna] ++ safe_exec_redirect [list git diff-tree -p --pretty $b] [list >$fnb] + } err]} { + error_popup [mc "Error writing commit to file: %s" $err] + return +@@ -9730,7 +9739,7 @@ proc exec_citool {tool_args {baseid {}}} { + } + } + +- eval exec git citool $tool_args & ++ safe_exec_redirect [concat git citool $tool_args] [list &] + + array unset env GIT_AUTHOR_* + array set env $save_env + +From 7a0493edda08fc0d8ee6d5489a50530c768646a1 Mon Sep 17 00:00:00 2001 +From: Johannes Sixt +Date: Sat, 29 Mar 2025 17:21:27 +0100 +Subject: [PATCH 06/41] gitk: sanitize 'exec' arguments: redirections and + background + +Convert 'exec' calls that both redirect output to a file and run the +process in the background. 'safe_exec_redirect' can take both these +"redirections" in the second argument simultaneously. + +Signed-off-by: Johannes Sixt + +Signed-off-by: Taylor Blau +--- + gitk | 5 ++--- + 1 file changed, 2 insertions(+), 3 deletions(-) + +diff --git a/gitk-git/gitk b/gitk-git/gitk +index 218f61fa28f223..c0d793f05dc045 100755 +--- a/gitk-git/gitk ++++ b/gitk-git/gitk +@@ -9363,8 +9363,7 @@ proc mkpatchgo {} { + set newid [$patchtop.tosha1 get] + set fname [$patchtop.fname get] + set cmd [diffcmd [list $oldid $newid] -p] +- lappend cmd >$fname & +- if {[catch {eval exec $cmd} err]} { ++ if {[catch {safe_exec_redirect $cmd [list >$fname &]} err]} { + error_popup "[mc "Error creating patch:"] $err" $patchtop + } + catch {destroy $patchtop} +@@ -9553,7 +9552,7 @@ proc wrcomgo {} { + set id [$wrcomtop.sha1 get] + set cmd "echo $id | [$wrcomtop.cmd get]" + set fname [$wrcomtop.fname get] +- if {[catch {exec sh -c $cmd >$fname &} err]} { ++ if {[catch {safe_exec_redirect [list sh -c $cmd] [list >$fname &]} err]} { + error_popup "[mc "Error writing commit:"] $err" $wrcomtop + } + catch {destroy $wrcomtop} + +From 30846b43060c3d57575b59b9aaa80c4bd1688171 Mon Sep 17 00:00:00 2001 +From: Johannes Sixt +Date: Sat, 29 Mar 2025 17:35:19 +0100 +Subject: [PATCH 07/41] gitk: sanitize 'exec' arguments: redirect to process + +Convert one 'exec' call that sends output to a process (pipeline). +Fortunately, the command does not contain any variables. For this +reason, just treat it as a "redirection". + +Signed-off-by: Johannes Sixt + +Signed-off-by: Taylor Blau +--- + gitk | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/gitk-git/gitk b/gitk-git/gitk +index c0d793f05dc045..9673e56abda662 100755 +--- a/gitk-git/gitk ++++ b/gitk-git/gitk +@@ -43,7 +43,7 @@ proc safe_exec {cmd} { + # executes one command with redirections + # no pipelines are possible + # cmd is a list that specifies the command and its arguments +-# redir is a list that specifies redirections (output, background) ++# redir is a list that specifies redirections (output, background, constant(!) commands) + # calls `exec` and returns its value + proc safe_exec_redirect {cmd redir} { + eval exec [make_arglist_safe $cmd] $redir +@@ -9120,7 +9120,7 @@ proc getpatchid {id} { + if {![info exists patchids($id)]} { + set cmd [diffcmd [list $id] {-p --root}] + if {[catch { +- set x [eval exec $cmd | git patch-id] ++ set x [safe_exec_redirect $cmd [list | git patch-id]] + set patchids($id) [lindex $x 0] + }]} { + set patchids($id) "error" + +From fe32bf31b8d5dff523543700ab76ecbf423a6d6f Mon Sep 17 00:00:00 2001 +From: Johannes Sixt +Date: Thu, 20 Mar 2025 19:32:56 +0100 +Subject: [PATCH 08/41] 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 + +Signed-off-by: Taylor Blau +--- + gitk | 59 +++++++++++++++++++++++++++++++++-------------------------- + 1 file changed, 33 insertions(+), 26 deletions(-) + +diff --git a/gitk-git/gitk b/gitk-git/gitk +index 9673e56abda662..aba8ef63dc2b16 100755 +--- a/gitk-git/gitk ++++ b/gitk-git/gitk +@@ -59,6 +59,13 @@ proc safe_open_file {filename flags} { + open $filename $flags + } + ++# opens a command pipeline for reading ++# cmd is a list that specifies the command and its arguments ++# calls `open` and returns the file id ++proc safe_open_command {cmd} { ++ open |[make_arglist_safe $cmd] r ++} ++ + # End exec/open wrappers + + proc hasworktree {} { +@@ -186,7 +193,7 @@ proc unmerged_files {files} { + set mlist {} + set nr_unmerged 0 + if {[catch { +- set fd [open "| git ls-files -u" r] ++ set fd [safe_open_command {git ls-files -u}] + } err]} { + show_error {} . "[mc "Couldn't get list of unmerged files:"] $err" + exit 1 +@@ -1700,7 +1707,7 @@ proc do_readcommit {id} { + global tclencoding + + # Invoke git-log to handle automatic encoding conversion +- set fd [open [concat | git log --no-color --pretty=raw -1 $id] r] ++ set fd [safe_open_command [concat git log --no-color --pretty=raw -1 $id]] + # Read the results using i18n.logoutputencoding + fconfigure $fd -translation lf -eofchar {} + if {$tclencoding != {}} { +@@ -1836,7 +1843,7 @@ proc readrefs {} { + foreach v {tagids idtags headids idheads otherrefids idotherrefs} { + unset -nocomplain $v + } +- set refd [open [list | git show-ref -d] r] ++ set refd [safe_open_command [list git show-ref -d]] + if {$tclencoding != {}} { + fconfigure $refd -encoding $tclencoding + } +@@ -3729,7 +3736,7 @@ proc external_diff {} { + + if {$difffromfile ne {} && $difftofile ne {}} { + set cmd [list [shellsplit $extdifftool] $difffromfile $difftofile] +- if {[catch {set fl [open |$cmd r]} err]} { ++ if {[catch {set fl [safe_open_command $cmd]} err]} { + file delete -force $diffdir + error_popup "$extdifftool: [mc "command failed:"] $err" + } else { +@@ -3833,7 +3840,7 @@ proc external_blame_diff {} { + # Find the SHA1 ID of the blob for file $fname in the index + # at stage 0 or 2 + proc index_sha1 {fname} { +- set f [open [list | git ls-files -s $fname] r] ++ set f [safe_open_command [list git ls-files -s $fname]] + while {[gets $f line] >= 0} { + set info [lindex [split $line "\t"] 0] + set stage [lindex $info 2] +@@ -5311,8 +5318,8 @@ proc get_viewmainhead {view} { + global viewmainheadid vfilelimit viewinstances mainheadid + + catch { +- set rfd [open [concat | git rev-list -1 $mainheadid \ +- -- $vfilelimit($view)] r] ++ set rfd [safe_open_command [concat git rev-list -1 $mainheadid \ ++ -- $vfilelimit($view)]] + set j [reg_instance $rfd] + lappend viewinstances($view) $j + fconfigure $rfd -blocking 0 +@@ -5377,14 +5384,14 @@ proc dodiffindex {} { + if {!$showlocalchanges || !$hasworktree} return + incr lserial + if {[package vcompare $git_version "1.7.2"] >= 0} { +- set cmd "|git diff-index --cached --ignore-submodules=dirty HEAD" ++ set cmd "git diff-index --cached --ignore-submodules=dirty HEAD" + } else { +- set cmd "|git diff-index --cached HEAD" ++ set cmd "git diff-index --cached HEAD" + } + if {$vfilelimit($curview) ne {}} { + set cmd [concat $cmd -- $vfilelimit($curview)] + } +- set fd [open $cmd r] ++ set fd [safe_open_command $cmd] + fconfigure $fd -blocking 0 + set i [reg_instance $fd] + filerun $fd [list readdiffindex $fd $lserial $i] +@@ -5409,11 +5416,11 @@ proc readdiffindex {fd serial inst} { + } + + # now see if there are any local changes not checked in to the index +- set cmd "|git diff-files" ++ set cmd "git diff-files" + if {$vfilelimit($curview) ne {}} { + set cmd [concat $cmd -- $vfilelimit($curview)] + } +- set fd [open $cmd r] ++ set fd [safe_open_command $cmd] + fconfigure $fd -blocking 0 + set i [reg_instance $fd] + filerun $fd [list readdifffiles $fd $serial $i] +@@ -7705,13 +7712,13 @@ proc gettree {id} { + if {![info exists treefilelist($id)]} { + if {![info exists treepending]} { + if {$id eq $nullid} { +- set cmd [list | git ls-files] ++ set cmd [list git ls-files] + } elseif {$id eq $nullid2} { +- set cmd [list | git ls-files --stage -t] ++ set cmd [list git ls-files --stage -t] + } else { +- set cmd [list | git ls-tree -r $id] ++ set cmd [list git ls-tree -r $id] + } +- if {[catch {set gtf [open $cmd r]}]} { ++ if {[catch {set gtf [safe_open_command $cmd]}]} { + return + } + set treepending $id +@@ -7781,7 +7788,7 @@ proc showfile {f} { + } + } else { + set blob [lindex $treeidlist($diffids) $i] +- if {[catch {set bf [open [concat | git cat-file blob $blob] r]} err]} { ++ if {[catch {set bf [safe_open_command [concat git cat-file blob $blob]]} err]} { + puts "oops, error reading blob $blob: $err" + return + } +@@ -7976,7 +7983,7 @@ proc gettreediffs {ids} { + if {$limitdiffs && $vfilelimit($curview) ne {}} { + set cmd [concat $cmd -- $vfilelimit($curview)] + } +- if {[catch {set gdtf [open |$cmd r]}]} return ++ if {[catch {set gdtf [safe_open_command $cmd]}]} return + + set treepending $ids + set treediff {} +@@ -8096,7 +8103,7 @@ proc getblobdiffs {ids} { + if {$limitdiffs && $vfilelimit($curview) ne {}} { + set cmd [concat $cmd -- $vfilelimit($curview)] + } +- if {[catch {set bdf [open |$cmd r]} err]} { ++ if {[catch {set bdf [safe_open_command $cmd]} err]} { + error_popup [mc "Error getting diffs: %s" $err] + return + } +@@ -9223,7 +9230,7 @@ proc diffcommits {a b} { + return + } + if {[catch { +- set fd [open "| diff -U$diffcontext $fna $fnb" r] ++ set fd [safe_open_command "diff -U$diffcontext $fna $fnb"] + } err]} { + error_popup [mc "Error diffing commits: %s" $err] + return +@@ -10256,7 +10263,7 @@ proc getallcommits {} { + if {$allcwait} { + return + } +- set cmd [list | git rev-list --parents] ++ set cmd [list git rev-list --parents] + set allcupdate [expr {$seeds ne {}}] + if {!$allcupdate} { + set ids "--all" + +From 42a64b41a7a3d01a62f0f34f75bee2bbd00be46f Mon Sep 17 00:00:00 2001 +From: Johannes Sixt +Date: Thu, 20 Mar 2025 20:00:57 +0100 +Subject: [PATCH 09/41] gitk: sanitize 'open' arguments: simple commands with + redirections + +As in the previous commits, introduce a function that sanitizes +arguments intended for the process and in addition allows to pass +redirections, which are passed to Tcl's 'open' verbatim. + +Signed-off-by: Johannes Sixt + +Signed-off-by: Taylor Blau +--- + gitk | 18 +++++++++++++----- + 1 file changed, 13 insertions(+), 5 deletions(-) + +diff --git a/gitk-git/gitk b/gitk-git/gitk +index aba8ef63dc2b16..68d6bfd61f5f84 100755 +--- a/gitk-git/gitk ++++ b/gitk-git/gitk +@@ -66,6 +66,15 @@ proc safe_open_command {cmd} { + open |[make_arglist_safe $cmd] r + } + ++# opens a command pipeline for reading with redirections ++# cmd is a list that specifies the command and its arguments ++# redir is a list that specifies redirections ++# calls `open` and returns the file id ++proc safe_open_command_redirect {cmd redir} { ++ set cmd [make_arglist_safe $cmd] ++ open |[concat $cmd $redir] r ++} ++ + # End exec/open wrappers + + proc hasworktree {} { +@@ -9906,8 +9915,8 @@ proc resethead {} { + bind $w "grab $w; focus $w" + tkwait window $w + if {!$confirm_ok} return +- if {[catch {set fd [open \ +- [list | git reset --$resettype $rowmenuid 2>@1] r]} err]} { ++ if {[catch {set fd [safe_open_command_redirect \ ++ [list git reset --$resettype $rowmenuid] [list 2>@1]]} err]} { + error_popup $err + } else { + dohidelocalchanges +@@ -9978,7 +9987,7 @@ proc cobranch {} { + + # check the tree is clean first?? + set newhead $headmenuhead +- set command [list | git checkout] ++ set command [list git checkout] + if {[string match "remotes/*" $newhead]} { + set remote $newhead + set newhead [string range $newhead [expr [string last / $newhead] + 1] end] +@@ -9992,12 +10001,11 @@ proc cobranch {} { + } else { + lappend command $newhead + } +- lappend command 2>@1 + nowbusy checkout [mc "Checking out"] + update + dohidelocalchanges + if {[catch { +- set fd [open $command r] ++ set fd [safe_open_command_redirect $command [list 2>@1]] + } err]} { + notbusy checkout + error_popup $err + +From 2aeb4484a046a545fb540ba07397b25b13fe6881 Mon Sep 17 00:00:00 2001 +From: Johannes Sixt +Date: Fri, 21 Mar 2025 23:34:14 +0100 +Subject: [PATCH 10/41] gitk: sanitize 'open' arguments: simple commands, + readable and writable + +As in the previous commits, introduce a function that sanitizes +arguments and also keeps the returned file handle writable to pass +data to stdin. + +Signed-off-by: Johannes Sixt +Signed-off-by: Taylor Blau +--- + gitk | 11 +++++++++-- + 1 file changed, 9 insertions(+), 2 deletions(-) + +diff --git a/gitk-git/gitk b/gitk-git/gitk +index 68d6bfd61f5f84..22da6a811c2081 100755 +--- a/gitk-git/gitk ++++ b/gitk-git/gitk +@@ -66,6 +66,13 @@ proc safe_open_command {cmd} { + open |[make_arglist_safe $cmd] r + } + ++# opens a command pipeline for reading and writing ++# cmd is a list that specifies the command and its arguments ++# calls `open` and returns the file id ++proc safe_open_command_rw {cmd} { ++ open |[make_arglist_safe $cmd] r+ ++} ++ + # opens a command pipeline for reading with redirections + # cmd is a list that specifies the command and its arguments + # redir is a list that specifies redirections +@@ -4897,8 +4904,8 @@ proc do_file_hl {serial} { + # must be "containing:", i.e. we're searching commit info + return + } +- set cmd [concat | git diff-tree -r -s --stdin $gdtargs] +- set filehighlight [open $cmd r+] ++ set cmd [concat git diff-tree -r -s --stdin $gdtargs] ++ set filehighlight [safe_open_command_rw $cmd] + fconfigure $filehighlight -blocking 0 + filerun $filehighlight readfhighlight + set fhl_list {} + +From 79a3ef53143f75450a828f4bc4e9dd3d4f2bb5ba Mon Sep 17 00:00:00 2001 +From: Johannes Sixt +Date: Sun, 23 Mar 2025 22:34:11 +0100 +Subject: [PATCH 11/41] gitk: collect construction of blameargs into a single + conditional + +The command line to invoke 'git blame' for a single line is constructed +using several if-conditionals, each with the same condition +{$from_index new {}}. Merge all of them into a single conditional. +This requires to duplicate significant parts of the command, but it +helps the next change, where we will have to deal with a nested list +structure. + +Signed-off-by: Johannes Sixt +Signed-off-by: Taylor Blau +--- + gitk | 14 ++++++-------- + 1 file changed, 6 insertions(+), 8 deletions(-) + +diff --git a/gitk-git/gitk b/gitk-git/gitk +index 22da6a811c2081..2e37ddea969d1e 100755 +--- a/gitk-git/gitk ++++ b/gitk-git/gitk +@@ -3967,17 +3967,15 @@ proc show_line_source {} { + } + set line [lindex $h 1] + } +- set blameargs {} ++ set blamefile [file join $cdup $flist_menu_file] + if {$from_index ne {}} { +- lappend blameargs | git cat-file blob $from_index +- } +- lappend blameargs | git blame -p -L$line,+1 +- if {$from_index ne {}} { +- lappend blameargs --contents - ++ set blameargs [list \ ++ | git cat-file blob $from_index \ ++ | git blame -p -L$line,+1 --contents - -- $blamefile] + } else { +- lappend blameargs $id ++ set blameargs [list \ ++ | git blame -p -L$line,+1 $id -- $blamefile] + } +- lappend blameargs -- [file join $cdup $flist_menu_file] + if {[catch { + set f [open $blameargs r] + } err]} { + +From 026c397d911cde55924d7eb1311d0fd6e2e105d5 Mon Sep 17 00:00:00 2001 +From: Johannes Sixt +Date: Sun, 23 Mar 2025 22:45:39 +0100 +Subject: [PATCH 12/41] gitk: sanitize 'open' arguments: command pipeline + +As in the earlier commits, introduce a function that constructs a +pipeline of commands after sanitizing the arguments. + +Signed-off-by: Johannes Sixt + +Signed-off-by: Taylor Blau +--- + gitk | 19 +++++++++++++++---- + 1 file changed, 15 insertions(+), 4 deletions(-) + +diff --git a/gitk-git/gitk b/gitk-git/gitk +index 2e37ddea969d1e..9bd226ec8330e5 100755 +--- a/gitk-git/gitk ++++ b/gitk-git/gitk +@@ -82,6 +82,17 @@ proc safe_open_command_redirect {cmd redir} { + open |[concat $cmd $redir] r + } + ++# opens a pipeline with several commands for reading ++# cmds is a list of lists, each of which specifies a command and its arguments ++# calls `open` and returns the file id ++proc safe_open_pipeline {cmds} { ++ set cmd {} ++ foreach subcmd $cmds { ++ set cmd [concat $cmd | [make_arglist_safe $subcmd]] ++ } ++ open $cmd r ++} ++ + # End exec/open wrappers + + proc hasworktree {} { +@@ -3970,14 +3981,14 @@ proc show_line_source {} { + set blamefile [file join $cdup $flist_menu_file] + if {$from_index ne {}} { + set blameargs [list \ +- | git cat-file blob $from_index \ +- | git blame -p -L$line,+1 --contents - -- $blamefile] ++ [list git cat-file blob $from_index] \ ++ [list git blame -p -L$line,+1 --contents - -- $blamefile]] + } else { + set blameargs [list \ +- | git blame -p -L$line,+1 $id -- $blamefile] ++ [list git blame -p -L$line,+1 $id -- $blamefile]] + } + if {[catch { +- set f [open $blameargs r] ++ set f [safe_open_pipeline $blameargs] + } err]} { + error_popup [mc "Couldn't start git blame: %s" $err] + return + +From 37b9230226db2a3a81df2e92a44ea655076cd0c4 Mon Sep 17 00:00:00 2001 +From: Mark Levedahl +Date: Thu, 3 Apr 2025 10:26:21 -0400 +Subject: [PATCH 14/41] git-gui: _which, only add .exe suffix if not present + +The _which function finds executables on $PATH, and adds .exe on Windows +unless -script was given. However, win32.tcl executes "wscript.exe" +and "cscript.exe", both of which fail as _which adds .exe to both. This +is already fixed in git-gui released by Git for Windows. Do so here. + +Signed-off-by: Mark Levedahl +Signed-off-by: Johannes Sixt +Signed-off-by: Taylor Blau +--- + git-gui/git-gui.sh | 3 +++ + 1 file changed, 3 insertions(+) + +diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh +index 3e5907a4609b15..2e079b6ab138a1 100755 +--- a/git-gui/git-gui.sh ++++ b/git-gui/git-gui.sh +@@ -101,6 +101,9 @@ proc _which {what args} { + + if {[is_Windows] && [lsearch -exact $args -script] >= 0} { + set suffix {} ++ } elseif {[is_Windows] && [string match *$_search_exe [string tolower $what]]} { ++ # The search string already has the file extension ++ set suffix {} + } else { + set suffix $_search_exe + } + +From c5c32781c99bfa9d8b7c51b4a1ad66a9fa1e6bfe Mon Sep 17 00:00:00 2001 +From: Mark Levedahl +Date: Wed, 2 Apr 2025 11:23:03 -0400 +Subject: [PATCH 15/41] git-gui: use [is_Windows], not bad _shellpath + +Commit 7d076d56757c (git-gui: handle shell script text filters when +loading for blame, 2011-12-09) added open_cmd_pipe, with special +handling for Windows detected by seeing that _shellpath does not +point to an executable shell. That is bad practice, and is broken by +the next commit that assures _shellpath is valid on all platforms. + +Fix this by using [is_Windows] as done for all Windows specific code. + +Signed-off-by: Mark Levedahl +Signed-off-by: Johannes Sixt + +Signed-off-by: Taylor Blau +--- + git-gui/git-gui.sh | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh +index 2e079b6ab138a1..3135116169f8ff 100755 +--- a/git-gui/git-gui.sh ++++ b/git-gui/git-gui.sh +@@ -543,7 +543,7 @@ proc is_shellscript {filename} { + # scripts specifically otherwise just call the filter command. + proc open_cmd_pipe {cmd path} { + global env +- if {![file executable [shellpath]]} { ++ if {[is_Windows]} { + set exe [auto_execok [lindex $cmd 0]] + if {[is_shellscript [lindex $exe 0]]} { + set run [linsert [auto_execok sh] end -c "$cmd \"\$0\"" $path] + +From 10637fc327fe9d3afd19a11ed64bd9e1c7a9c6b5 Mon Sep 17 00:00:00 2001 +From: Mark Levedahl +Date: Tue, 1 Apr 2025 11:45:06 -0400 +Subject: [PATCH 16/41] git-gui: make _shellpath usable on startup + +Since commit d5257fb3c1de (git-gui: handle textconv filter on +Windows and in development, 2010-08-07), git-gui will search for a +usable shell if _shellpath is not configured, and on Windows may +resort to using auto_execok to find 'sh'. While this was intended for +development use, checks are insufficient to assure a proper +configuration when deployed where _shellpath is always set, but might +not give a usable shell. + +Let's make this more robust by only searching if _shellpath was not +defined, and then using only our restricted search functions. +Furthermore, we should convert to a Windows path on Windows. Always +check for a valid shell on startup, meaning an absolute path to an +executable, aborting if these conditions are not met. + +Signed-off-by: Mark Levedahl +Signed-off-by: Johannes Sixt + +Signed-off-by: Taylor Blau +--- + git-gui/git-gui.sh | 38 ++++++++++++++++++++++++++++++-------- + 1 file changed, 30 insertions(+), 8 deletions(-) + +diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh +index 3135116169f8ff..d56610c8922fd8 100755 +--- a/git-gui/git-gui.sh ++++ b/git-gui/git-gui.sh +@@ -307,15 +307,37 @@ if {$_trace >= 0} { + # branches). + set _last_merged_branch {} + +-proc shellpath {} { +- global _shellpath env +- if {[string match @@* $_shellpath]} { +- if {[info exists env(SHELL)]} { +- return $env(SHELL) +- } else { +- return /bin/sh +- } ++# for testing, allow unconfigured _shellpath ++if {[string match @@* $_shellpath]} { ++ if {[info exists env(SHELL)]} { ++ set _shellpath $env(SHELL) ++ } else { ++ set _shellpath /bin/sh + } ++} ++ ++if {[is_Windows]} { ++ set _shellpath [exec cygpath -m $_shellpath] ++} ++ ++if {![file executable $_shellpath] || \ ++ !([file pathtype $_shellpath] eq {absolute})} { ++ set errmsg "The defined shell ('$_shellpath') is not usable, \ ++ it must be an absolute path to an executable." ++ puts stderr $errmsg ++ ++ catch {wm withdraw .} ++ tk_messageBox \ ++ -icon error \ ++ -type ok \ ++ -title "git-gui: configuration error" \ ++ -message $errmsg ++ exit 1 ++} ++ ++ ++proc shellpath {} { ++ global _shellpath + return $_shellpath + } + + +From 4774c704d20e50ad710f65756099c3eedbfbe789 Mon Sep 17 00:00:00 2001 +From: Mark Levedahl +Date: Wed, 20 Sep 2023 17:56:14 -0400 +Subject: [PATCH 17/41] git-gui: remove Tcl 8.4 workaround on 2>@1 redirection + +Since b792230 ("git-gui: Show a progress meter for checking out files", +2007-07-08), git-gui includes a workaround for Tcl that does not support +using 2>@1 to redirect stderr to stdout. Tcl added such support in +8.4.7, released in 2004, and this is fully supported in all 8.5 +releases. + +As git-gui has a hard-coded requirement for Tcl >= 8.5, the workaround +is no longer needed. Delete it. + +Signed-off-by: Mark Levedahl +Signed-off-by: Johannes Sixt + +Signed-off-by: Taylor Blau +--- + git-gui/git-gui.sh | 21 +++------------------ + 1 file changed, 3 insertions(+), 18 deletions(-) + +diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh +index 3e5907a4609b15..ca1362aa19f3b5 100755 +--- a/git-gui/git-gui.sh ++++ b/git-gui/git-gui.sh +@@ -584,24 +584,9 @@ proc git {args} { + proc _open_stdout_stderr {cmd} { + _trace_exec $cmd + if {[catch { +- set fd [open [concat [list | ] $cmd] r] +- } err]} { +- if { [lindex $cmd end] eq {2>@1} +- && $err eq {can not find channel named "1"} +- } { +- # Older versions of Tcl 8.4 don't have this 2>@1 IO +- # redirect operator. Fallback to |& cat for those. +- # The command was not actually started, so its safe +- # to try to start it a second time. +- # +- set fd [open [concat \ +- [list | ] \ +- [lrange $cmd 0 end-1] \ +- [list |& cat] \ +- ] r] +- } else { +- error $err +- } ++ set fd [open [concat [list | ] $cmd] r] ++ } err]} { ++ error $err + } + fconfigure $fd -eofchar {} + return $fd + +From 02dd866ba9dfe6b6090b351450894489ca80311f Mon Sep 17 00:00:00 2001 +From: Mark Levedahl +Date: Sun, 6 Apr 2025 18:20:14 -0400 +Subject: [PATCH 18/41] git-gui: use only the configured shell + +git-gui has a few places where a bare "sh" is passed to exec, meaning +that the first instance of "sh" on $PATH will be used rather than the +shell configured. This violates expectations that the configured shell +is being used. Let's use [shellpath] everywhere. + +Signed-off-by: Mark Levedahl +Signed-off-by: Johannes Sixt +Signed-off-by: Taylor Blau +--- + git-gui/lib/sshkey.tcl | 3 ++- + git-gui/lib/tools.tcl | 4 ++-- + 2 files changed, 4 insertions(+), 3 deletions(-) + +diff --git a/git-gui/lib/sshkey.tcl b/git-gui/lib/sshkey.tcl +index 589ff8f78aba82..c0c5d1dad8252d 100644 +--- a/git-gui/lib/sshkey.tcl ++++ b/git-gui/lib/sshkey.tcl +@@ -83,7 +83,8 @@ proc make_ssh_key {w} { + set sshkey_title [mc "Generating..."] + $w.header.gen configure -state disabled + +- set cmdline [list sh -c {echo | ssh-keygen -q -t rsa -f ~/.ssh/id_rsa 2>&1}] ++ set cmdline [list [shellpath] -c \ ++ {echo | ssh-keygen -q -t rsa -f ~/.ssh/id_rsa 2>&1}] + + if {[catch { set sshkey_fd [_open_stdout_stderr $cmdline] } err]} { + error_popup [mc "Could not start ssh-keygen:\n\n%s" $err] +diff --git a/git-gui/lib/tools.tcl b/git-gui/lib/tools.tcl +index 413f1a170079e0..b86f72ed167568 100644 +--- a/git-gui/lib/tools.tcl ++++ b/git-gui/lib/tools.tcl +@@ -110,14 +110,14 @@ proc tools_exec {fullname} { + + set cmdline $repo_config(guitool.$fullname.cmd) + if {[is_config_true "guitool.$fullname.noconsole"]} { +- tools_run_silent [list sh -c $cmdline] \ ++ tools_run_silent [list [shellpath] -c $cmdline] \ + [list tools_complete $fullname {}] + } else { + regsub {/} $fullname { / } title + set w [console::new \ + [mc "Tool: %s" $title] \ + [mc "Running: %s" $cmdline]] +- console::exec $w [list sh -c $cmdline] \ ++ console::exec $w [list [shellpath] -c $cmdline] \ + [list tools_complete $fullname $w] + } + + +From f9a2e8a38f524c04fc493548303488dc180b25bd Mon Sep 17 00:00:00 2001 +From: Mark Levedahl +Date: Fri, 2 May 2025 11:39:55 -0400 +Subject: [PATCH 19/41] git-gui: remove HEAD detachment implementation for git + < 1.5.3 + +git-gui provides an implementation to detach HEAD on Git versions prior +to 1.5.3. Nobody should be using such an old version anymore. +(Moreover, since 0730a5a3a, git-gui requires git v2.36 or later). +Keep only the code for modern Git. + +Signed-off-by: Mark Levedahl +[j6t: message tweaked] +Signed-off-by: Johannes Sixt + +Signed-off-by: Taylor Blau +--- + git-gui/lib/checkout_op.tcl | 14 ++------------ + 1 file changed, 2 insertions(+), 12 deletions(-) + +diff --git a/git-gui/lib/checkout_op.tcl b/git-gui/lib/checkout_op.tcl +index 21ea768d8036c0..5f7011078abbcf 100644 +--- a/git-gui/lib/checkout_op.tcl ++++ b/git-gui/lib/checkout_op.tcl +@@ -510,18 +510,8 @@ method _update_repo_state {} { + delete_this + } + +-git-version proc _detach_HEAD {log new} { +- >= 1.5.3 { +- git update-ref --no-deref -m $log HEAD $new +- } +- default { +- set p [gitdir HEAD] +- file delete $p +- set fd [open $p w] +- fconfigure $fd -translation lf -encoding utf-8 +- puts $fd $new +- close $fd +- } ++proc _detach_HEAD {log new} { ++ git update-ref --no-deref -m $log HEAD $new + } + + method _confirm_reset {cur} { + +From 4eb9b1157b6d597ba3f599e2ebbfd4c6d8504073 Mon Sep 17 00:00:00 2001 +From: Johannes Sixt +Date: Sun, 18 May 2025 16:08:06 +0200 +Subject: [PATCH 20/41] git-gui: remove special treatment of Windows from + open_cmd_pipe + +Commit 7d076d56757c (git-gui: handle shell script text filters when +loading for blame, 2011-12-09) added open_cmd_pipe to run text +conversion in support of blame, with special handling for shell +scripts on Windows. To determine whether the command is a shell +script, 'lindex' is used to pick off the first token from the command. +However, cmd is actually a command string taken from .gitconfig +literally and is not necessarily a syntactically correct Tcl list. +Hence, it cannot be processed by 'lindex' and 'lrange' reliably. +Pass the command string to the shell just like on non-Windows +platforms to avoid the potentially incorrect treatment. + +A use of 'auto_execok' is removed by this change. This function is +dangerous on Windows, because it searches programs in the current +directory. Delegating the path lookup to the shell is safe, because +/bin/sh and /bin/bash follow POSIX on all platforms, including the +Git for Windows port. + +A possible regression is that the old code, given filter command of +'foo', could find 'foo.bat' as a script, and not just bare 'foo', or +'foo.exe'. This rewrite requires explicitly giving the suffix if it is +not .exe. + +This part of Git GUI can be exercised using + + git gui blame -- some.file + +while some.file has a textconv filter configured and has unstaged +modifications. + +Helped-by: Mark Levedahl +Signed-off-by: Johannes Sixt + +Signed-off-by: Taylor Blau +--- + git-gui/git-gui.sh | 19 +++++-------------- + 1 file changed, 5 insertions(+), 14 deletions(-) + +diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh +index d56610c8922fd8..cb0c02e5b8f439 100755 +--- a/git-gui/git-gui.sh ++++ b/git-gui/git-gui.sh +@@ -559,22 +559,13 @@ proc is_shellscript {filename} { + return [expr {$magic eq "#!"}] + } + +-# Run a command connected via pipes on stdout. ++# Run a shell command connected via pipes on stdout. + # This is for use with textconv filters and uses sh -c "..." to allow it to +-# contain a command with arguments. On windows we must check for shell +-# scripts specifically otherwise just call the filter command. ++# contain a command with arguments. We presume this ++# to be a shellscript that the configured shell (/bin/sh by default) knows ++# how to run. + proc open_cmd_pipe {cmd path} { +- global env +- if {[is_Windows]} { +- set exe [auto_execok [lindex $cmd 0]] +- if {[is_shellscript [lindex $exe 0]]} { +- set run [linsert [auto_execok sh] end -c "$cmd \"\$0\"" $path] +- } else { +- set run [concat $exe [lrange $cmd 1 end] $path] +- } +- } else { +- set run [list [shellpath] -c "$cmd \"\$0\"" $path] +- } ++ set run [list [shellpath] -c "$cmd \"\$0\"" $path] + return [open |$run r] + } + + +From 8255167b26003767b0ab50f498ffec33f80c2ef2 Mon Sep 17 00:00:00 2001 +From: Johannes Sixt +Date: Sat, 3 May 2025 13:37:35 +0200 +Subject: [PATCH 21/41] git-gui: remove git config --list handling for git < + 1.5.3 + +git-gui uses `git config --null --list` to parse configuration. Git +versions prior to 1.5.3 do not have --null and need different treatment. +Nobody should be using such an old version anymore. (Moreover, since +0730a5a3a, git-gui requires git v2.36 or later). Keep only the code for +modern Git. + +Signed-off-by: Johannes Sixt + +Signed-off-by: Taylor Blau +--- + git-gui/git-gui.sh | 69 ++++++++++++++++++------------------------------------ + 1 file changed, 23 insertions(+), 46 deletions(-) + +diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh +index ca1362aa19f3b5..2e325b042a068a 100755 +--- a/git-gui/git-gui.sh ++++ b/git-gui/git-gui.sh +@@ -1083,53 +1083,30 @@ unset -nocomplain idx fd + ## + ## config file parsing + +-git-version proc _parse_config {arr_name args} { +- >= 1.5.3 { +- upvar $arr_name arr +- array unset arr +- set buf {} +- catch { +- set fd_rc [eval \ +- [list git_read config] \ +- $args \ +- [list --null --list]] +- fconfigure $fd_rc -translation binary -encoding utf-8 +- set buf [read $fd_rc] +- close $fd_rc +- } +- foreach line [split $buf "\0"] { +- if {[regexp {^([^\n]+)\n(.*)$} $line line name value]} { +- if {[is_many_config $name]} { +- lappend arr($name) $value +- } else { +- set arr($name) $value +- } +- } elseif {[regexp {^([^\n]+)$} $line line name]} { +- # no value given, but interpreting them as +- # boolean will be handled as true +- set arr($name) {} +- } +- } +- } +- default { +- upvar $arr_name arr +- array unset arr +- catch { +- set fd_rc [eval [list git_read config --list] $args] +- while {[gets $fd_rc line] >= 0} { +- if {[regexp {^([^=]+)=(.*)$} $line line name value]} { +- if {[is_many_config $name]} { +- lappend arr($name) $value +- } else { +- set arr($name) $value +- } +- } elseif {[regexp {^([^=]+)$} $line line name]} { +- # no value given, but interpreting them as +- # boolean will be handled as true +- set arr($name) {} +- } ++proc _parse_config {arr_name args} { ++ upvar $arr_name arr ++ array unset arr ++ set buf {} ++ catch { ++ set fd_rc [eval \ ++ [list git_read config] \ ++ $args \ ++ [list --null --list]] ++ fconfigure $fd_rc -translation binary -encoding utf-8 ++ set buf [read $fd_rc] ++ close $fd_rc ++ } ++ foreach line [split $buf "\0"] { ++ if {[regexp {^([^\n]+)\n(.*)$} $line line name value]} { ++ if {[is_many_config $name]} { ++ lappend arr($name) $value ++ } else { ++ set arr($name) $value + } +- close $fd_rc ++ } elseif {[regexp {^([^\n]+)$} $line line name]} { ++ # no value given, but interpreting them as ++ # boolean will be handled as true ++ set arr($name) {} + } + } + } + +From 2c66188b123795e4083594ee682dcf012540bee2 Mon Sep 17 00:00:00 2001 +From: Mark Levedahl +Date: Fri, 4 Apr 2025 16:55:59 -0400 +Subject: [PATCH 22/41] git-gui: remove unused proc is_shellscript + +Commit 7d076d56757c (git-gui: handle shell script text filters when +loading for blame, 2011-12-09) added is_shellscript to test if a file +is executable by the shell, used only when searching for textconv +filters. The previous commit rearranged the tests for finding such +filters, and removed the only user of is_shellscript. Remove this +function. + +Signed-off-by: Mark Levedahl +Signed-off-by: Johannes Sixt +Signed-off-by: Taylor Blau +--- + git-gui/git-gui.sh | 10 ---------- + 1 file changed, 10 deletions(-) + +diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh +index cb0c02e5b8f439..72da2443e5eb6e 100755 +--- a/git-gui/git-gui.sh ++++ b/git-gui/git-gui.sh +@@ -549,16 +549,6 @@ proc _git_cmd {name} { + return $v + } + +-# Test a file for a hashbang to identify executable scripts on Windows. +-proc is_shellscript {filename} { +- if {![file exists $filename]} {return 0} +- set f [open $filename r] +- fconfigure $f -encoding binary +- set magic [read $f 2] +- close $f +- return [expr {$magic eq "#!"}] +-} +- + # Run a shell command connected via pipes on stdout. + # This is for use with textconv filters and uses sh -c "..." to allow it to + # contain a command with arguments. We presume this + +From c2e8904258544f3d79dc4e96d1269c0ad8124db3 Mon Sep 17 00:00:00 2001 +From: Johannes Sixt +Date: Mon, 21 Apr 2025 17:07:10 +0200 +Subject: [PATCH 23/41] 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 + +Signed-off-by: Taylor Blau +--- + git-gui/git-gui.sh | 36 ++++++++++++++++++++++++------------ + git-gui/lib/blame.tcl | 2 +- + git-gui/lib/choose_repository.tcl | 14 +++++++------- + git-gui/lib/choose_rev.tcl | 2 +- + git-gui/lib/commit.tcl | 4 ++-- + git-gui/lib/diff.tcl | 2 +- + git-gui/lib/merge.tcl | 2 +- + git-gui/lib/mergetool.tcl | 2 +- + git-gui/lib/remote.tcl | 6 +++--- + git-gui/lib/shortcut.tcl | 4 ++-- + git-gui/lib/sshkey.tcl | 2 +- + 11 files changed, 44 insertions(+), 32 deletions(-) + +diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh +index 2e325b042a068a..52b463c45fb582 100755 +--- a/git-gui/git-gui.sh ++++ b/git-gui/git-gui.sh +@@ -170,6 +170,18 @@ proc open {args} { + uplevel 1 real_open $args + } + ++# Wrap open to sanitize arguments ++ ++proc safe_open_file {filename flags} { ++ # a file name starting with "|" would attempt to run a process ++ # but such a file name must be treated as a relative path ++ # hide the "|" behind "./" ++ if {[string index $filename 0] eq "|"} { ++ set filename [file join . $filename] ++ } ++ open $filename $flags ++} ++ + ###################################################################### + ## + ## locate our library +@@ -494,7 +506,7 @@ proc _git_cmd {name} { + # Tcl on Windows doesn't know it. + # + set p [gitexec git-$name] +- set f [open $p r] ++ set f [safe_open_file $p r] + set s [gets $f] + close $f + +@@ -683,7 +695,7 @@ proc sq {value} { + proc load_current_branch {} { + global current_branch is_detached + +- set fd [open [gitdir HEAD] r] ++ set fd [safe_open_file [gitdir HEAD] r] + fconfigure $fd -translation binary -encoding utf-8 + if {[gets $fd ref] < 1} { + set ref {} +@@ -1045,7 +1057,7 @@ You are using [git-version]: + ## configure our library + + set idx [file join $oguilib tclIndex] +-if {[catch {set fd [open $idx r]} err]} { ++if {[catch {set fd [safe_open_file $idx r]} err]} { + catch {wm withdraw .} + tk_messageBox \ + -icon error \ +@@ -1382,7 +1394,7 @@ proc repository_state {ctvar hdvar mhvar} { + set merge_head [gitdir MERGE_HEAD] + if {[file exists $merge_head]} { + set ct merge +- set fd_mh [open $merge_head r] ++ set fd_mh [safe_open_file $merge_head r] + while {[gets $fd_mh line] >= 0} { + lappend mh $line + } +@@ -1530,7 +1542,7 @@ proc load_message {file {encoding {}}} { + + set f [gitdir $file] + if {[file isfile $f]} { +- if {[catch {set fd [open $f r]}]} { ++ if {[catch {set fd [safe_open_file $f r]}]} { + return 0 + } + fconfigure $fd -eofchar {} +@@ -1554,23 +1566,23 @@ proc run_prepare_commit_msg_hook {} { + # it will be .git/MERGE_MSG (merge), .git/SQUASH_MSG (squash), or an + # empty file but existent file. + +- set fd_pcm [open [gitdir PREPARE_COMMIT_MSG] a] ++ set fd_pcm [safe_open_file [gitdir PREPARE_COMMIT_MSG] a] + + if {[file isfile [gitdir MERGE_MSG]]} { + set pcm_source "merge" +- set fd_mm [open [gitdir MERGE_MSG] r] ++ set fd_mm [safe_open_file [gitdir MERGE_MSG] r] + fconfigure $fd_mm -encoding utf-8 + puts -nonewline $fd_pcm [read $fd_mm] + close $fd_mm + } elseif {[file isfile [gitdir SQUASH_MSG]]} { + set pcm_source "squash" +- set fd_sm [open [gitdir SQUASH_MSG] r] ++ set fd_sm [safe_open_file [gitdir SQUASH_MSG] r] + fconfigure $fd_sm -encoding utf-8 + puts -nonewline $fd_pcm [read $fd_sm] + close $fd_sm + } elseif {[file isfile [get_config commit.template]]} { + set pcm_source "template" +- set fd_sm [open [get_config commit.template] r] ++ set fd_sm [safe_open_file [get_config commit.template] r] + fconfigure $fd_sm -encoding utf-8 + puts -nonewline $fd_pcm [read $fd_sm] + close $fd_sm +@@ -2271,7 +2283,7 @@ proc do_quit {{rc {1}}} { + if {![string match amend* $commit_type] + && $msg ne {}} { + catch { +- set fd [open $save w] ++ set fd [safe_open_file $save w] + fconfigure $fd -encoding utf-8 + puts -nonewline $fd $msg + close $fd +@@ -4032,7 +4044,7 @@ if {[winfo exists $ui_comm]} { + } + } elseif {$m} { + catch { +- set fd [open [gitdir GITGUI_BCK] w] ++ set fd [safe_open_file [gitdir GITGUI_BCK] w] + fconfigure $fd -encoding utf-8 + puts -nonewline $fd $msg + close $fd +diff --git a/git-gui/lib/blame.tcl b/git-gui/lib/blame.tcl +index 8441e109be3282..e70a079fa76f85 100644 +--- a/git-gui/lib/blame.tcl ++++ b/git-gui/lib/blame.tcl +@@ -481,7 +481,7 @@ method _load {jump} { + if {$do_textconv ne 0} { + set fd [open_cmd_pipe $textconv $path] + } else { +- set fd [open $path r] ++ set fd [safe_open_file $path r] + } + fconfigure $fd -eofchar {} + } else { +diff --git a/git-gui/lib/choose_repository.tcl b/git-gui/lib/choose_repository.tcl +index d23abedcb36fd9..ef7ad7c1750a97 100644 +--- a/git-gui/lib/choose_repository.tcl ++++ b/git-gui/lib/choose_repository.tcl +@@ -641,8 +641,8 @@ method _do_clone2 {} { + set pwd [pwd] + if {[catch { + file mkdir [gitdir objects info] +- set f_in [open [file join $objdir info alternates] r] +- set f_cp [open [gitdir objects info alternates] w] ++ set f_in [safe_open_file [file join $objdir info alternates] r] ++ set f_cp [safe_open_file [gitdir objects info alternates] w] + fconfigure $f_in -translation binary -encoding binary + fconfigure $f_cp -translation binary -encoding binary + cd $objdir +@@ -727,7 +727,7 @@ method _do_clone2 {} { + [cb _do_clone_tags] + } + shared { +- set fd [open [gitdir objects info alternates] w] ++ set fd [safe_open_file [gitdir objects info alternates] w] + fconfigure $fd -translation binary + puts $fd $objdir + close $fd +@@ -760,8 +760,8 @@ method _copy_files {objdir tocopy} { + } + foreach p $tocopy { + if {[catch { +- set f_in [open [file join $objdir $p] r] +- set f_cp [open [file join .git objects $p] w] ++ set f_in [safe_open_file [file join $objdir $p] r] ++ set f_cp [safe_open_file [file join .git objects $p] w] + fconfigure $f_in -translation binary -encoding binary + fconfigure $f_cp -translation binary -encoding binary + +@@ -823,7 +823,7 @@ method _clone_refs {} { + {--format=list %(refname) %(objectname) %(*objectname)}] + cd $pwd + +- set fd [open [gitdir packed-refs] w] ++ set fd [safe_open_file [gitdir packed-refs] w] + fconfigure $fd -translation binary + puts $fd "# pack-refs with: peeled" + while {[gets $fd_in line] >= 0} { +@@ -877,7 +877,7 @@ method _do_clone_full_end {ok} { + + set HEAD {} + if {[file exists [gitdir FETCH_HEAD]]} { +- set fd [open [gitdir FETCH_HEAD] r] ++ set fd [safe_open_file [gitdir FETCH_HEAD] r] + while {[gets $fd line] >= 0} { + if {[regexp "^(.{40})\t\t" $line line HEAD]} { + break +diff --git a/git-gui/lib/choose_rev.tcl b/git-gui/lib/choose_rev.tcl +index 6dae7937d589c1..7cf9e160fa3b54 100644 +--- a/git-gui/lib/choose_rev.tcl ++++ b/git-gui/lib/choose_rev.tcl +@@ -579,7 +579,7 @@ method _reflog_last {name} { + + set last {} + if {[catch {set last [file mtime [gitdir $name]]}] +- && ![catch {set g [open [gitdir logs $name] r]}]} { ++ && ![catch {set g [safe_open_file [gitdir logs $name] r]}]} { + fconfigure $g -translation binary + while {[gets $g line] >= 0} { + if {[regexp {> ([1-9][0-9]*) } $line line when]} { +diff --git a/git-gui/lib/commit.tcl b/git-gui/lib/commit.tcl +index 11379f8ad355e2..8d135845a5ec4f 100644 +--- a/git-gui/lib/commit.tcl ++++ b/git-gui/lib/commit.tcl +@@ -225,7 +225,7 @@ A good commit message has the following format: + # -- Build the message file. + # + set msg_p [gitdir GITGUI_EDITMSG] +- set msg_wt [open $msg_p w] ++ set msg_wt [safe_open_file $msg_p w] + fconfigure $msg_wt -translation lf + setup_commit_encoding $msg_wt + puts $msg_wt $msg +@@ -409,7 +409,7 @@ A rescan will be automatically started now. + if {$commit_type ne {normal}} { + append reflogm " ($commit_type)" + } +- set msg_fd [open $msg_p r] ++ set msg_fd [safe_open_file $msg_p r] + setup_commit_encoding $msg_fd 1 + gets $msg_fd subject + close $msg_fd +diff --git a/git-gui/lib/diff.tcl b/git-gui/lib/diff.tcl +index 871ad488c2a1c0..f089fdc46b2197 100644 +--- a/git-gui/lib/diff.tcl ++++ b/git-gui/lib/diff.tcl +@@ -202,7 +202,7 @@ proc show_other_diff {path w m cont_info} { + set sz [string length $content] + } + file { +- set fd [open $path r] ++ set fd [safe_open_file $path r] + fconfigure $fd \ + -eofchar {} \ + -encoding [get_path_encoding $path] +diff --git a/git-gui/lib/merge.tcl b/git-gui/lib/merge.tcl +index 664803cf3fd14c..897dc2a286b8b8 100644 +--- a/git-gui/lib/merge.tcl ++++ b/git-gui/lib/merge.tcl +@@ -93,7 +93,7 @@ method _start {} { + set spec [$w_rev get_tracking_branch] + set cmit [$w_rev get_commit] + +- set fh [open [gitdir FETCH_HEAD] w] ++ set fh [safe_open_file [gitdir FETCH_HEAD] w] + fconfigure $fh -translation lf + if {$spec eq {}} { + set remote . +diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl +index e688b016ef6c9d..f2fa4393057d66 100644 +--- a/git-gui/lib/mergetool.tcl ++++ b/git-gui/lib/mergetool.tcl +@@ -293,7 +293,7 @@ proc merge_tool_get_stages {target stages} { + foreach fname $stages { + if {$merge_stages($i) eq {}} { + file delete $fname +- catch { close [open $fname w] } ++ catch { close [safe_open_file $fname w] } + } else { + # A hack to support autocrlf properly + git checkout-index -f --stage=$i -- $target +diff --git a/git-gui/lib/remote.tcl b/git-gui/lib/remote.tcl +index ef77ed7399c5b0..a733a0f36e6e21 100644 +--- a/git-gui/lib/remote.tcl ++++ b/git-gui/lib/remote.tcl +@@ -75,7 +75,7 @@ proc load_all_remotes {} { + + foreach name $all_remotes { + catch { +- set fd [open [file join $rm_dir $name] r] ++ set fd [safe_open_file [file join $rm_dir $name] r] + while {[gets $fd line] >= 0} { + if {[regexp {^URL:[ ]*(.+)$} $line line url]} { + set remote_url($name) $url +@@ -145,7 +145,7 @@ proc add_fetch_entry {r} { + } + } else { + catch { +- set fd [open [gitdir remotes $r] r] ++ set fd [safe_open_file [gitdir remotes $r] r] + while {[gets $fd n] >= 0} { + if {[regexp {^Pull:[ \t]*([^:]+):} $n]} { + set enable 1 +@@ -182,7 +182,7 @@ proc add_push_entry {r} { + } + } else { + catch { +- set fd [open [gitdir remotes $r] r] ++ set fd [safe_open_file [gitdir remotes $r] r] + while {[gets $fd n] >= 0} { + if {[regexp {^Push:[ \t]*([^:]+):} $n]} { + set enable 1 +diff --git a/git-gui/lib/shortcut.tcl b/git-gui/lib/shortcut.tcl +index 674a41f5e0c868..9be79b2e895ff6 100644 +--- a/git-gui/lib/shortcut.tcl ++++ b/git-gui/lib/shortcut.tcl +@@ -83,7 +83,7 @@ proc do_macosx_app {} { + + file mkdir $MacOS + +- set fd [open [file join $Contents Info.plist] w] ++ set fd [safe_open_file [file join $Contents Info.plist] w] + puts $fd { + + +@@ -108,7 +108,7 @@ proc do_macosx_app {} { + } + close $fd + +- set fd [open $exe w] ++ set fd [safe_open_file $exe w] + puts $fd "#!/bin/sh" + foreach name [lsort [array names env]] { + set value $env($name) +diff --git a/git-gui/lib/sshkey.tcl b/git-gui/lib/sshkey.tcl +index 589ff8f78aba82..2e006cb8ca6706 100644 +--- a/git-gui/lib/sshkey.tcl ++++ b/git-gui/lib/sshkey.tcl +@@ -7,7 +7,7 @@ proc find_ssh_key {} { + ~/.ssh/id_rsa.pub ~/.ssh/identity.pub + } { + if {[file exists $name]} { +- set fh [open $name r] ++ set fh [safe_open_file $name r] + set cont [read $fh] + close $fh + return [list $name $cont] + +From 411cd493cb9998c2ba1c17dd98c3d3d4b121a1dd Mon Sep 17 00:00:00 2001 +From: Mark Levedahl +Date: Wed, 2 Apr 2025 17:37:27 -0400 +Subject: [PATCH 24/41] git-gui: avoid auto_execok for git-bash menu item + +On Windows, git-gui offers to open a git-bash session for the current +repository from the menu, but uses [auto_execok start] to get the +command to actually run that shell. + +The code for auto_execok, in /usr/share/tcl8.6/tcl.init, has 'start' in +the 'shellBuiltins' list for cmd.exe on Windows: as a result, +auto_execok does not actually search for start, meaning this usage is +technically ok with auto_execok now. However, leaving this use of +auto_execok in place will just induce confusion about why a known unsafe +function is being used on Windows. Instead, let's switch to using our +known safe _which function that looks only in $PATH, excluding the +current working directory. + +Signed-off-by: Mark Levedahl +Signed-off-by: Johannes Sixt +Signed-off-by: Taylor Blau +--- + git-gui/git-gui.sh | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh +index 72da2443e5eb6e..3bfe4364c72672 100755 +--- a/git-gui/git-gui.sh ++++ b/git-gui/git-gui.sh +@@ -2769,7 +2769,7 @@ if {[is_Windows]} { + } + .mbar.repository add command \ + -label [mc "Git Bash"] \ +- -command {eval exec [auto_execok start] $cmdLine} ++ -command {eval exec [list [_which cmd] /c start] $cmdLine} + } + + if {[is_Windows] || ![is_bare]} { + +From 4f3e0a4bcef2c6caff68f96137d9914c5f2f98c2 Mon Sep 17 00:00:00 2001 +From: Johannes Sixt +Date: Mon, 21 Apr 2025 18:14:54 +0200 +Subject: [PATCH 25/41] 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 + +Signed-off-by: Taylor Blau +--- + git-gui/git-gui.sh | 42 ++++++++++++++++++++++++++++++++++++------ + git-gui/lib/diff.tcl | 2 +- + git-gui/lib/shortcut.tcl | 8 ++++---- + git-gui/lib/win32.tcl | 9 +++++---- + 4 files changed, 46 insertions(+), 15 deletions(-) + +diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh +index 52b463c45fb582..a6be30e295bb41 100755 +--- a/git-gui/git-gui.sh ++++ b/git-gui/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/git-gui/lib/diff.tcl b/git-gui/lib/diff.tcl +index f089fdc46b2197..cfa8a414d30d16 100644 +--- a/git-gui/lib/diff.tcl ++++ b/git-gui/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/git-gui/lib/shortcut.tcl b/git-gui/lib/shortcut.tcl +index 9be79b2e895ff6..d437ea69339dc6 100644 +--- a/git-gui/lib/shortcut.tcl ++++ b/git-gui/lib/shortcut.tcl +@@ -30,11 +30,11 @@ proc do_cygwin_shortcut {} { + global argv0 _gitworktree + + if {[catch { +- set desktop [exec cygpath \ ++ set desktop [safe_exec [list cygpath \ + --windows \ + --absolute \ + --long-name \ +- --desktop] ++ --desktop]] + }]} { + set desktop . + } +diff --git a/git-gui/lib/win32.tcl b/git-gui/lib/win32.tcl +index db91ab84a56d79..3aedae2f135dbc 100644 +--- a/git-gui/lib/win32.tcl ++++ b/git-gui/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] + } + +From 00c7aa86e905175476e0dff149d570b48c34c8f1 Mon Sep 17 00:00:00 2001 +From: Mark Levedahl +Date: Thu, 3 Apr 2025 00:37:08 -0400 +Subject: [PATCH 26/41] git-gui: avoid auto_execok in do_windows_shortcut + +git-gui on Windows uses auto_execok to locate git-gui.exe, +which performs the same flawed search as does the builtin exec. +Use _which instead, performing a safe PATH lookup. + +Signed-off-by: Mark Levedahl +Signed-off-by: Johannes Sixt +Signed-off-by: Taylor Blau +--- + git-gui/lib/shortcut.tcl | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/git-gui/lib/shortcut.tcl b/git-gui/lib/shortcut.tcl +index 674a41f5e0c868..263f4899c9e94a 100644 +--- a/git-gui/lib/shortcut.tcl ++++ b/git-gui/lib/shortcut.tcl +@@ -12,7 +12,7 @@ proc do_windows_shortcut {} { + set fn ${fn}.lnk + } + # Use git-gui.exe if available (ie: git-for-windows) +- set cmdLine [auto_execok git-gui.exe] ++ set cmdLine [list [_which git-gui]] + if {$cmdLine eq {}} { + set cmdLine [list [info nameofexecutable] \ + [file normalize $::argv0]] + +From e883ceb1222c29f8a2325e1b4c2778b5259a3725 Mon Sep 17 00:00:00 2001 +From: Johannes Sixt +Date: Sat, 26 Apr 2025 18:46:06 +0200 +Subject: [PATCH 27/41] git-gui: sanitize 'exec' arguments: background + +As in the previous commits, introduce a function that sanitizes +arguments intended for the process, but runs the process in the +background. Convert 'exec' calls to use this new function. + +Signed-off-by: Johannes Sixt +Signed-off-by: Taylor Blau +--- + git-gui/git-gui.sh | 28 +++++++++++++++++++--------- + 1 file changed, 19 insertions(+), 9 deletions(-) + +diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh +index a6be30e295bb41..6ecddfdd0afb6c 100755 +--- a/git-gui/git-gui.sh ++++ b/git-gui/git-gui.sh +@@ -200,6 +200,14 @@ proc safe_exec {cmd} { + eval exec [make_arglist_safe $cmd] + } + ++# executes one command in the background ++# 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_bg {cmd} { ++ eval exec [make_arglist_safe $cmd] & ++} ++ + proc safe_open_file {filename flags} { + # a file name starting with "|" would attempt to run a process + # but such a file name must be treated as a relative path +@@ -2202,7 +2210,7 @@ proc do_gitk {revs {is_submodule false}} { + unset env(GIT_DIR) + unset env(GIT_WORK_TREE) + } +- eval exec $cmd $revs "--" "--" & ++ safe_exec_bg [concat $cmd $revs "--" "--"] + + set env(GIT_DIR) $_gitdir + set env(GIT_WORK_TREE) $_gitworktree +@@ -2239,7 +2247,7 @@ proc do_git_gui {} { + set pwd [pwd] + cd $current_diff_path + +- eval exec $exe gui & ++ safe_exec_bg [concat $exe gui] + + set env(GIT_DIR) $_gitdir + set env(GIT_WORK_TREE) $_gitworktree +@@ -2270,16 +2278,18 @@ proc get_explorer {} { + + proc do_explore {} { + global _gitworktree +- set explorer [get_explorer] +- eval exec $explorer [list [file nativename $_gitworktree]] & ++ set cmd [get_explorer] ++ lappend cmd [file nativename $_gitworktree] ++ safe_exec_bg $cmd + } + + # Open file relative to the working tree by the default associated app. + proc do_file_open {file} { + global _gitworktree +- set explorer [get_explorer] ++ set cmd [get_explorer] + set full_file_path [file join $_gitworktree $file] +- exec $explorer [file nativename $full_file_path] & ++ lappend cmd [file nativename $full_file_path] ++ safe_exec_bg $cmd + } + + set is_quitting 0 + +From 676c49583f063c6cb92e5b38eb6576929cebf1ff Mon Sep 17 00:00:00 2001 +From: Mark Levedahl +Date: Mon, 7 Apr 2025 17:12:56 -0400 +Subject: [PATCH 28/41] git-gui: cleanup git-bash menu item + +git-gui on Git for Windows creates a menu item to start a git-bash +session for the current repository. This menu-item works as desired when +git-gui is installed in the Git for Windows (g4w) distribution, but +not when run from a different location such as normally done in +development. The reason is that git-bash's location is known to be +'/git-bash' in the Unix pathname space known to MSYS, but this is not +known in the Windows pathname space. Instead, git-gui derives a pathname +for git-bash assuming it is at a known relative location. + +If git-gui is run from a different directory than assumed in g4w, the +relative location changes, and git-gui resorts to running a generic bash +login session in a Windows console. + +But, the MSYS system underlying Git for Windows includes the 'cygpath' +utility to convert between Unix and Windows pathnames. Let's use this so +git-bash's Windows pathname is determined directly from /git-bash. + +Signed-off-by: Mark Levedahl +Signed-off-by: Johannes Sixt +Signed-off-by: Taylor Blau +--- + git-gui/git-gui.sh | 13 ++++++------- + 1 file changed, 6 insertions(+), 7 deletions(-) + +diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh +index 3bfe4364c72672..570c236f57f3c7 100755 +--- a/git-gui/git-gui.sh ++++ b/git-gui/git-gui.sh +@@ -2759,17 +2759,16 @@ if {![is_bare]} { + + if {[is_Windows]} { + # Use /git-bash.exe if available +- set normalized [file normalize $::argv0] +- regsub "/mingw../libexec/git-core/git-gui$" \ +- $normalized "/git-bash.exe" cmdLine +- if {$cmdLine != $normalized && [file exists $cmdLine]} { +- set cmdLine [list "Git Bash" $cmdLine &] ++ set _git_bash [exec cygpath -m /git-bash.exe] ++ if {[file executable $_git_bash]} { ++ set _bash_cmdline [list "Git Bash" $_git_bash] + } else { +- set cmdLine [list "Git Bash" bash --login -l &] ++ set _bash_cmdline [list "Git Bash" bash --login -l] + } + .mbar.repository add command \ + -label [mc "Git Bash"] \ +- -command {eval exec [list [_which cmd] /c start] $cmdLine} ++ -command {safe_exec_bg [concat [list [_which cmd] /c start] $_bash_cmdline]} ++ unset _git_bash + } + + if {[is_Windows] || ![is_bare]} { + +From 23ba43256b421c322af9b99150fb324575175bb0 Mon Sep 17 00:00:00 2001 +From: Johannes Sixt +Date: Sat, 3 May 2025 11:52:35 +0200 +Subject: [PATCH 29/41] git-gui: remove option --stderr from git_read + +Some callers of git_read want to redirect stderr of the invoked command +to stdout. The function offers option --stderr for this purpose. +However, the option only appends 2>@1 to the commands. The callers can +do that themselves. In git-gui/lib/console.tcl we even have a caller that +already knew implictly what --stderr does behind the scenes. + +This is a preparation for a later change where we want to make git_read +non-variadic. Then it cannot have optional leading arguments. + +Signed-off-by: Johannes Sixt +Signed-off-by: Taylor Blau +--- + git-gui/git-gui.sh | 4 ---- + git-gui/lib/checkout_op.tcl | 3 ++- + git-gui/lib/choose_repository.tcl | 3 ++- + git-gui/lib/console.tcl | 4 ++-- + git-gui/lib/merge.tcl | 2 +- + 5 files changed, 7 insertions(+), 9 deletions(-) + +diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh +index 6ecddfdd0afb6c..890a172fc49b33 100755 +--- a/git-gui/git-gui.sh ++++ b/git-gui/git-gui.sh +@@ -651,10 +651,6 @@ proc git_read {args} { + _lappend_nice opt + } + +- --stderr { +- lappend args 2>@1 +- } +- + default { + break + } +diff --git a/git-gui/lib/checkout_op.tcl b/git-gui/lib/checkout_op.tcl +index 5f7011078abbcf..31992f461bc889 100644 +--- a/git-gui/lib/checkout_op.tcl ++++ b/git-gui/lib/checkout_op.tcl +@@ -345,13 +345,14 @@ method _readtree {} { + [mc "Updating working directory to '%s'..." [_name $this]] \ + [mc "files checked out"]] + +- set fd [git_read --stderr read-tree \ ++ set fd [git_read read-tree \ + -m \ + -u \ + -v \ + --exclude-per-directory=.gitignore \ + $HEAD \ + $new_hash \ ++ 2>@1 \ + ] + fconfigure $fd -blocking 0 -translation binary + fileevent $fd readable [cb _readtree_wait $fd $status_bar_operation] +diff --git a/git-gui/lib/choose_repository.tcl b/git-gui/lib/choose_repository.tcl +index ef7ad7c1750a97..7bd738e51e0a9e 100644 +--- a/git-gui/lib/choose_repository.tcl ++++ b/git-gui/lib/choose_repository.tcl +@@ -953,12 +953,13 @@ method _do_clone_checkout {HEAD} { + [mc "files"]] + + set readtree_err {} +- set fd [git_read --stderr read-tree \ ++ set fd [git_read read-tree \ + -m \ + -u \ + -v \ + HEAD \ + HEAD \ ++ 2>@1 \ + ] + fconfigure $fd -blocking 0 -translation binary + fileevent $fd readable [cb _readtree_wait $fd] +diff --git a/git-gui/lib/console.tcl b/git-gui/lib/console.tcl +index bb6b9c889e20a3..c7f20b87cbf4bb 100644 +--- a/git-gui/lib/console.tcl ++++ b/git-gui/lib/console.tcl +@@ -91,10 +91,10 @@ method _init {} { + } + + method exec {cmd {after {}}} { ++ lappend cmd 2>@1 + if {[lindex $cmd 0] eq {git}} { +- set fd_f [eval git_read --stderr [lrange $cmd 1 end]] ++ set fd_f [eval git_read [lrange $cmd 1 end]] + } else { +- lappend cmd 2>@1 + set fd_f [_open_stdout_stderr $cmd] + } + fconfigure $fd_f -blocking 0 -translation binary +diff --git a/git-gui/lib/merge.tcl b/git-gui/lib/merge.tcl +index 897dc2a286b8b8..e97c91eab67ae2 100644 +--- a/git-gui/lib/merge.tcl ++++ b/git-gui/lib/merge.tcl +@@ -239,7 +239,7 @@ Continue with resetting the current changes?"] + } + + if {[ask_popup $op_question] eq {yes}} { +- set fd [git_read --stderr read-tree --reset -u -v HEAD] ++ set fd [git_read read-tree --reset -u -v HEAD 2>@1] + fconfigure $fd -blocking 0 -translation binary + set status_bar_operation [$::main_status \ + start \ + +From 8fe7861c5185248a5786e87af71e29000cd4f214 Mon Sep 17 00:00:00 2001 +From: Mark Levedahl +Date: Fri, 11 Apr 2025 10:08:52 -0400 +Subject: [PATCH 30/41] git-gui: assure PATH has only absolute elements. + +Since 8f23432b38d9 (windows: ignore empty `PATH` elements, 2022-11-23), +git-gui excises all empty paths from $PATH, but still allows '.' or +other relative paths, which can also allow executing code from the +repository. Let's remove anything except absolute elements. While here, +let's remove duplicated elements, which are very common on Windows: +only the first such item can do anything except waste time repeating a +search. + +Signed-off-by: Mark Levedahl +Signed-off-by: Johannes Sixt + +Signed-off-by: Taylor Blau +--- + git-gui/git-gui.sh | 20 ++++++++++++++++---- + 1 file changed, 16 insertions(+), 4 deletions(-) + +diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh +index 570c236f57f3c7..9ccd88893030f0 100755 +--- a/git-gui/git-gui.sh ++++ b/git-gui/git-gui.sh +@@ -88,10 +88,22 @@ proc _which {what args} { + set gitguidir [file dirname [info script]] + regsub -all ";" $gitguidir "\\;" gitguidir + set env(PATH) "$gitguidir;$env(PATH)" +- set _search_path [split $env(PATH) {;}] +- # Skip empty `PATH` elements +- set _search_path [lsearch -all -inline -not -exact \ +- $_search_path ""] ++ ++ set _path_seen [dict create] ++ foreach p [split $env(PATH) {;}] { ++ # Keep only absolute paths, getting rid of ., empty, etc. ++ if {[file pathtype $p] ne {absolute}} { ++ continue ++ } ++ # Keep only the first occurence of any duplicates. ++ set norm_p [file normalize $p] ++ if {[dict exists $_path_seen $norm_p]} { ++ continue ++ } ++ dict set _path_seen $norm_p 1 ++ lappend _search_path $norm_p ++ } ++ unset _path_seen + set _search_exe .exe + } else { + set _search_path [split $env(PATH) :] + +From aa42e87ef4ee9d84bd2fdb5e56de2ac2b61575d9 Mon Sep 17 00:00:00 2001 +From: Johannes Sixt +Date: Sat, 3 May 2025 13:11:21 +0200 +Subject: [PATCH 31/41] git-gui: break out a separate function git_read_nice + +There are two callers of git_read that request special treatment using +option --nice. Rewrite them to call a new function git_read_nice that +does the special treatment. Now we can remove all option treatment from +git_read. + +git_write has the same capability, but there are no callers that +request --nice. Remove the feature without substitution. + +This is a preparation for a later change where we want to make git_read +and friends non-variadic. Then it cannot have optional arguments. + +Signed-off-by: Johannes Sixt + +Signed-off-by: Taylor Blau +--- + git-gui/git-gui.sh | 41 +++++++++-------------------------------- + git-gui/lib/blame.tcl | 2 +- + git-gui/lib/diff.tcl | 2 +- + 3 files changed, 11 insertions(+), 34 deletions(-) + +diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh +index 890a172fc49b33..28113220af8770 100755 +--- a/git-gui/git-gui.sh ++++ b/git-gui/git-gui.sh +@@ -643,22 +643,16 @@ proc _open_stdout_stderr {cmd} { + } + + proc git_read {args} { +- set opt [list] +- +- while {1} { +- switch -- [lindex $args 0] { +- --nice { +- _lappend_nice opt +- } ++ set cmdp [_git_cmd [lindex $args 0]] ++ set args [lrange $args 1 end] + +- default { +- break +- } ++ return [_open_stdout_stderr [concat $cmdp $args]] ++} + +- } ++proc git_read_nice {args} { ++ set opt [list] + +- set args [lrange $args 1 end] +- } ++ _lappend_nice opt + + set cmdp [_git_cmd [lindex $args 0]] + set args [lrange $args 1 end] +@@ -667,28 +661,11 @@ proc git_read {args} { + } + + proc git_write {args} { +- set opt [list] +- +- while {1} { +- switch -- [lindex $args 0] { +- --nice { +- _lappend_nice opt +- } +- +- default { +- break +- } +- +- } +- +- set args [lrange $args 1 end] +- } +- + set cmdp [_git_cmd [lindex $args 0]] + set args [lrange $args 1 end] + +- _trace_exec [concat $opt $cmdp $args] +- return [open [concat [list | ] $opt $cmdp $args] w] ++ _trace_exec [concat $cmdp $args] ++ return [open [concat [list | ] $cmdp $args] w] + } + + proc githook_read {hook_name args} { +diff --git a/git-gui/lib/blame.tcl b/git-gui/lib/blame.tcl +index e70a079fa76f85..ceb2330266592b 100644 +--- a/git-gui/lib/blame.tcl ++++ b/git-gui/lib/blame.tcl +@@ -617,7 +617,7 @@ method _exec_blame {cur_w cur_d options cur_s} { + } + + lappend options -- $path +- set fd [eval git_read --nice blame $options] ++ set fd [eval git_read_nice blame $options] + fconfigure $fd -blocking 0 -translation lf -encoding utf-8 + fileevent $fd readable [cb _read_blame $fd $cur_w $cur_d] + set current_fd $fd +diff --git a/git-gui/lib/diff.tcl b/git-gui/lib/diff.tcl +index cfa8a414d30d16..ce1bad69007bae 100644 +--- a/git-gui/lib/diff.tcl ++++ b/git-gui/lib/diff.tcl +@@ -338,7 +338,7 @@ proc start_show_diff {cont_info {add_opts {}}} { + } + } + +- if {[catch {set fd [eval git_read --nice $cmd]} err]} { ++ if {[catch {set fd [eval git_read_nice $cmd]} err]} { + set diff_active 0 + unlock_index + ui_status [mc "Unable to display %s" [escape_path $path]] + +From 384b1409e8ba05bb908979a3f6aaa45bf93ac3c9 Mon Sep 17 00:00:00 2001 +From: Mark Levedahl +Date: Fri, 11 Apr 2025 10:47:04 -0400 +Subject: [PATCH 32/41] git-gui: sanitize $PATH on all platforms + +Since 8f23432b38d9 (windows: ignore empty `PATH` elements, 2022-11-23), +git-gui removes empty elements from $PATH, and a prior commit made this +remove all non-absolute elements from $PATH. But, this happens only on +Windows. Unsafe $PATH elements in $PATH are possible on all platforms. +Let's sanitize $PATH on all platforms to have consistent behavior. If a +user really wants the current repository on $PATH, they can add its +absolute name to $PATH. + +Signed-off-by: Mark Levedahl +Signed-off-by: Johannes Sixt + +Signed-off-by: Taylor Blau +--- + git-gui/git-gui.sh | 64 +++++++++++++++++++++++++++++------------------------- + 1 file changed, 34 insertions(+), 30 deletions(-) + +diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh +index 9ccd88893030f0..217aeb9ce3a7b5 100755 +--- a/git-gui/git-gui.sh ++++ b/git-gui/git-gui.sh +@@ -77,46 +77,43 @@ proc is_Cygwin {} { + + ###################################################################### + ## +-## PATH lookup ++## PATH lookup. Sanitize $PATH, assure exec/open use only that + +-set _search_path {} +-proc _which {what args} { +- global env _search_exe _search_path ++if {[is_Windows]} { ++ set _path_sep {;} ++ set _search_exe .exe ++} else { ++ set _path_sep {:} ++ set _search_exe {} ++} + +- if {$_search_path eq {}} { +- if {[is_Cygwin] && [regexp {^(/|\.:)} $env(PATH)]} { +- set _search_path [split [exec cygpath \ +- --windows \ +- --path \ +- --absolute \ +- $env(PATH)] {;}] +- set _search_exe .exe +- } elseif {[is_Windows]} { +- set gitguidir [file dirname [info script]] +- regsub -all ";" $gitguidir "\\;" gitguidir +- set env(PATH) "$gitguidir;$env(PATH)" +- +- set _path_seen [dict create] +- foreach p [split $env(PATH) {;}] { +- # Keep only absolute paths, getting rid of ., empty, etc. +- if {[file pathtype $p] ne {absolute}} { +- continue +- } +- # Keep only the first occurence of any duplicates. +- set norm_p [file normalize $p] +- if {[dict exists $_path_seen $norm_p]} { +- continue +- } +- dict set _path_seen $norm_p 1 +- lappend _search_path $norm_p +- } +- unset _path_seen +- set _search_exe .exe +- } else { +- set _search_path [split $env(PATH) :] +- set _search_exe {} +- } ++if {[is_Windows]} { ++ set gitguidir [file dirname [info script]] ++ regsub -all ";" $gitguidir "\\;" gitguidir ++ set env(PATH) "$gitguidir;$env(PATH)" ++} ++ ++set _search_path {} ++set _path_seen [dict create] ++foreach p [split $env(PATH) $_path_sep] { ++ # Keep only absolute paths, getting rid of ., empty, etc. ++ if {[file pathtype $p] ne {absolute}} { ++ continue + } ++ # Keep only the first occurence of any duplicates. ++ set norm_p [file normalize $p] ++ if {[dict exists $_path_seen $norm_p]} { ++ continue ++ } ++ dict set _path_seen $norm_p 1 ++ lappend _search_path $norm_p ++} ++unset _path_seen ++ ++set env(PATH) [join $_search_path $_path_sep] ++ ++proc _which {what args} { ++ global _search_exe _search_path + + if {[is_Windows] && [lsearch -exact $args -script] >= 0} { + set suffix {} + +From a1ccd2512072cf52835050f4c97a4fba9f0ec8f9 Mon Sep 17 00:00:00 2001 +From: Mark Levedahl +Date: Fri, 11 Apr 2025 10:58:20 -0400 +Subject: [PATCH 35/41] git-gui: override exec and open only on Windows + +Since aae9560a355d (Work around Tcl's default `PATH` lookup, +2022-11-23), git-gui overrides exec and open on all platforms. But, +this was done in response to Tcl adding elements to $PATH on Windows, +while exec, open, and auto_execok honor $PATH as given on all other +platforms. + +Let's do the override only on Windows, restoring others to using their +native exec and open. These honor the sanitized $PATH as that is written +out to env(PATH) in a previous commit. auto_execok is also safe on these +platforms, so can be used for _which. + +Signed-off-by: Mark Levedahl +Signed-off-by: Johannes Sixt + +Signed-off-by: Taylor Blau +--- + git-gui/git-gui.sh | 120 +++++++++++++++++++++++++++++------------------------ + 1 file changed, 65 insertions(+), 55 deletions(-) + +diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh +index 217aeb9ce3a7b5..21c3858d2e9af1 100755 +--- a/git-gui/git-gui.sh ++++ b/git-gui/git-gui.sh +@@ -112,81 +112,91 @@ unset _path_seen + + set env(PATH) [join $_search_path $_path_sep] + +-proc _which {what args} { +- global _search_exe _search_path +- +- if {[is_Windows] && [lsearch -exact $args -script] >= 0} { +- set suffix {} +- } elseif {[is_Windows] && [string match *$_search_exe [string tolower $what]]} { +- # The search string already has the file extension +- set suffix {} +- } else { +- set suffix $_search_exe +- } ++if {[is_Windows]} { ++ proc _which {what args} { ++ global _search_exe _search_path ++ ++ if {[lsearch -exact $args -script] >= 0} { ++ set suffix {} ++ } elseif {[string match *$_search_exe [string tolower $what]]} { ++ # The search string already has the file extension ++ set suffix {} ++ } else { ++ set suffix $_search_exe ++ } + +- foreach p $_search_path { +- set p [file join $p $what$suffix] +- if {[file exists $p]} { +- return [file normalize $p] ++ foreach p $_search_path { ++ set p [file join $p $what$suffix] ++ if {[file exists $p]} { ++ return [file normalize $p] ++ } + } ++ return {} + } +- return {} +-} + +-proc sanitize_command_line {command_line from_index} { +- set i $from_index +- while {$i < [llength $command_line]} { +- set cmd [lindex $command_line $i] +- if {[file pathtype $cmd] ne "absolute"} { +- set fullpath [_which $cmd] +- if {$fullpath eq ""} { +- throw {NOT-FOUND} "$cmd not found in PATH" ++ proc sanitize_command_line {command_line from_index} { ++ set i $from_index ++ while {$i < [llength $command_line]} { ++ set cmd [lindex $command_line $i] ++ if {[llength [file split $cmd]] < 2} { ++ set fullpath [_which $cmd] ++ if {$fullpath eq ""} { ++ throw {NOT-FOUND} "$cmd not found in PATH" ++ } ++ lset command_line $i $fullpath ++ } ++ ++ # handle piped commands, e.g. `exec A | B` ++ for {incr i} {$i < [llength $command_line]} {incr i} { ++ if {[lindex $command_line $i] eq "|"} { ++ incr i ++ break ++ } + } +- lset command_line $i $fullpath + } ++ return $command_line ++ } ++ ++ # Override `exec` to avoid unsafe PATH lookup ++ ++ rename exec real_exec + +- # handle piped commands, e.g. `exec A | B` +- for {incr i} {$i < [llength $command_line]} {incr i} { +- if {[lindex $command_line $i] eq "|"} { ++ proc exec {args} { ++ # skip options ++ for {set i 0} {$i < [llength $args]} {incr i} { ++ set arg [lindex $args $i] ++ if {$arg eq "--"} { + incr i + break + } ++ if {[string range $arg 0 0] ne "-"} { ++ break ++ } + } ++ set args [sanitize_command_line $args $i] ++ uplevel 1 real_exec $args + } +- return $command_line +-} + +-# Override `exec` to avoid unsafe PATH lookup ++ # Override `open` to avoid unsafe PATH lookup + +-rename exec real_exec ++ rename open real_open + +-proc exec {args} { +- # skip options +- for {set i 0} {$i < [llength $args]} {incr i} { +- set arg [lindex $args $i] +- if {$arg eq "--"} { +- incr i +- break +- } +- if {[string range $arg 0 0] ne "-"} { +- break ++ proc open {args} { ++ set arg0 [lindex $args 0] ++ if {[string range $arg0 0 0] eq "|"} { ++ set command_line [string trim [string range $arg0 1 end]] ++ lset args 0 "| [sanitize_command_line $command_line 0]" + } ++ uplevel 1 real_open $args + } +- set args [sanitize_command_line $args $i] +- uplevel 1 real_exec $args +-} +- +-# Override `open` to avoid unsafe PATH lookup + +-rename open real_open ++} else { ++ # On non-Windows platforms, auto_execok, exec, and open are safe, and will ++ # use the sanitized search path. But, we need _which for these. + +-proc open {args} { +- set arg0 [lindex $args 0] +- if {[string range $arg0 0 0] eq "|"} { +- set command_line [string trim [string range $arg0 1 end]] +- lset args 0 "| [sanitize_command_line $command_line 0]" ++ proc _which {what args} { ++ return [lindex [auto_execok $what] 0] + } +- uplevel 1 real_open $args + } + + # Wrap exec/open to sanitize arguments + +From dc9ecb1aab1a3438fceeb44db67ddf8e8d938324 Mon Sep 17 00:00:00 2001 +From: Johannes Sixt +Date: Sat, 3 May 2025 13:24:48 +0200 +Subject: [PATCH 36/41] git-gui: convert git_read*, git_write to be + non-variadic + +We are going to treat command arguments and redirections differently to +avoid passing arguments that look like redirections to the command +accidentally. To do so, it will be necessary to know which arguments +are intentional redirections. As a preparation, convert git_read, +git_read_nice, and git_write to take just a single argument that is +the command in a list. Adjust all call sites accordingly. + +In the future, this argument will be the regular command arguments and +a second argument will be the redirection operations. + +Signed-off-by: Johannes Sixt + +Signed-off-by: Taylor Blau +--- + git-gui/git-gui.sh | 48 ++++++++++++++++++------------------ + git-gui/lib/blame.tcl | 10 ++++---- + git-gui/lib/branch.tcl | 6 ++--- + git-gui/lib/browser.tcl | 2 +- + git-gui/lib/checkout_op.tcl | 10 ++++---- + git-gui/lib/choose_repository.tcl | 8 +++--- + git-gui/lib/choose_rev.tcl | 6 ++--- + git-gui/lib/commit.tcl | 6 ++--- + git-gui/lib/console.tcl | 2 +- + git-gui/lib/database.tcl | 2 +- + git-gui/lib/diff.tcl | 8 +++--- + git-gui/lib/index.tcl | 8 +++--- + git-gui/lib/merge.tcl | 2 +- + git-gui/lib/mergetool.tcl | 2 +- + git-gui/lib/remote.tcl | 2 +- + git-gui/lib/remote_branch_delete.tcl | 2 +- + 16 files changed, 62 insertions(+), 62 deletions(-) + +diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh +index e10bf2a039295e..301c7647ecaec6 100755 +--- a/git-gui/git-gui.sh ++++ b/git-gui/git-gui.sh +@@ -621,7 +621,7 @@ proc _lappend_nice {cmd_var} { + } + + proc git {args} { +- set fd [eval [list git_read] $args] ++ set fd [git_read $args] + fconfigure $fd -translation binary -encoding utf-8 + set result [string trimright [read $fd] "\n"] + close $fd +@@ -642,58 +642,40 @@ proc _open_stdout_stderr {cmd} { + return $fd + } + +-proc git_read {args} { +- set cmdp [_git_cmd [lindex $args 0]] +- set args [lrange $args 1 end] ++proc git_read {cmd} { ++ set cmdp [_git_cmd [lindex $cmd 0]] ++ set cmd [lrange $cmd 1 end] + +- return [_open_stdout_stderr [concat $cmdp $args]] ++ return [_open_stdout_stderr [concat $cmdp $cmd]] + } + +-proc git_read_nice {args} { ++proc git_read_nice {cmd} { + set opt [list] + + _lappend_nice opt + +- set cmdp [_git_cmd [lindex $args 0]] +- set args [lrange $args 1 end] ++ set cmdp [_git_cmd [lindex $cmd 0]] ++ set cmd [lrange $cmd 1 end] + +- return [_open_stdout_stderr [concat $opt $cmdp $args]] ++ return [_open_stdout_stderr [concat $opt $cmdp $cmd]] + } + +-proc git_write {args} { +- set cmdp [_git_cmd [lindex $args 0]] +- set args [lrange $args 1 end] ++proc git_write {cmd} { ++ set cmdp [_git_cmd [lindex $cmd 0]] ++ set cmd [lrange $cmd 1 end] + +- _trace_exec [concat $cmdp $args] +- return [open [concat [list | ] $cmdp $args] w] ++ _trace_exec [concat $cmdp $cmd] ++ return [open [concat [list | ] $cmdp $cmd] w] + } + + proc githook_read {hook_name args} { +- set pchook [gitdir hooks $hook_name] +- lappend args 2>@1 +- +- # On Windows [file executable] might lie so we need to ask +- # the shell if the hook is executable. Yes that's annoying. +- # +- if {[is_Windows]} { +- upvar #0 _sh interp +- if {![info exists interp]} { +- set interp [_which sh] +- } +- if {$interp eq {}} { +- error "hook execution requires sh (not in PATH)" +- } +- +- set scr {if test -x "$1";then exec "$@";fi} +- set sh_c [list $interp -c $scr $interp $pchook] +- return [_open_stdout_stderr [concat $sh_c $args]] +- } +- +- if {[file executable $pchook]} { +- return [_open_stdout_stderr [concat [list $pchook] $args]] +- } +- +- return {} ++ set pchook [gitdir hooks $hook_name] ++ ++ if {[file executable $pchook]} { ++ return [safe_open_command [concat [list $pchook] $args] [list 2>@1]] ++ } ++ ++ return {} + } + + proc kill_file_process {fd} { +@@ -1110,10 +1110,10 @@ proc _parse_config {arr_name args} { + array unset arr + set buf {} + catch { +- set fd_rc [eval \ +- [list git_read config] \ ++ set fd_rc [git_read \ ++ [concat config \ + $args \ +- [list --null --list]] ++ --null --list]] + fconfigure $fd_rc -translation binary -encoding utf-8 + set buf [read $fd_rc] + close $fd_rc +@@ -1482,12 +1482,12 @@ proc rescan {after {honor_trustmtime 1}} { + } else { + set rescan_active 1 + ui_status [mc "Refreshing file status..."] +- set fd_rf [git_read update-index \ ++ set fd_rf [git_read [list update-index \ + -q \ + --unmerged \ + --ignore-missing \ + --refresh \ +- ] ++ ]] + fconfigure $fd_rf -blocking 0 -translation binary + fileevent $fd_rf readable \ + [list rescan_stage2 $fd_rf $after] +@@ -1527,11 +1527,11 @@ proc rescan_stage2 {fd after} { + set rescan_active 2 + ui_status [mc "Scanning for modified files ..."] + if {[git-version >= "1.7.2"]} { +- set fd_di [git_read diff-index --cached --ignore-submodules=dirty -z [PARENT]] ++ set fd_di [git_read [list diff-index --cached --ignore-submodules=dirty -z [PARENT]]] + } else { +- set fd_di [git_read diff-index --cached -z [PARENT]] ++ set fd_di [git_read [list diff-index --cached -z [PARENT]]] + } +- set fd_df [git_read diff-files -z] ++ set fd_df [git_read [list diff-files -z]] + + fconfigure $fd_di -blocking 0 -translation binary -encoding binary + fconfigure $fd_df -blocking 0 -translation binary -encoding binary +@@ -1540,7 +1540,7 @@ proc rescan_stage2 {fd after} { + fileevent $fd_df readable [list read_diff_files $fd_df $after] + + if {[is_config_true gui.displayuntracked]} { +- set fd_lo [eval git_read ls-files --others -z $ls_others] ++ set fd_lo [git_read [concat ls-files --others -z $ls_others]] + fconfigure $fd_lo -blocking 0 -translation binary -encoding binary + fileevent $fd_lo readable [list read_ls_others $fd_lo $after] + incr rescan_active +diff --git a/git-gui/lib/blame.tcl b/git-gui/lib/blame.tcl +index ceb2330266592b..d6fd8bea91ff53 100644 +--- a/git-gui/lib/blame.tcl ++++ b/git-gui/lib/blame.tcl +@@ -486,9 +486,9 @@ method _load {jump} { + fconfigure $fd -eofchar {} + } else { + if {$do_textconv ne 0} { +- set fd [git_read cat-file --textconv "$commit:$path"] ++ set fd [git_read [list cat-file --textconv "$commit:$path"]] + } else { +- set fd [git_read cat-file blob "$commit:$path"] ++ set fd [git_read [list cat-file blob "$commit:$path"]] + } + } + fconfigure $fd \ +@@ -617,7 +617,7 @@ method _exec_blame {cur_w cur_d options cur_s} { + } + + lappend options -- $path +- set fd [eval git_read_nice blame $options] ++ set fd [git_read_nice [concat blame $options]] + fconfigure $fd -blocking 0 -translation lf -encoding utf-8 + fileevent $fd readable [cb _read_blame $fd $cur_w $cur_d] + set current_fd $fd +@@ -986,7 +986,7 @@ method _showcommit {cur_w lno} { + if {[catch {set msg $header($cmit,message)}]} { + set msg {} + catch { +- set fd [git_read cat-file commit $cmit] ++ set fd [git_read [list cat-file commit $cmit]] + fconfigure $fd -encoding binary -translation lf + # By default commits are assumed to be in utf-8 + set enc utf-8 +@@ -1134,7 +1134,7 @@ method _blameparent {} { + } else { + set diffcmd [list diff-tree --unified=0 $cparent $cmit -- $new_path] + } +- if {[catch {set fd [eval git_read $diffcmd]} err]} { ++ if {[catch {set fd [git_read $diffcmd]} err]} { + $status_operation stop [mc "Unable to display parent"] + error_popup [strcat [mc "Error loading diff:"] "\n\n$err"] + return +diff --git a/git-gui/lib/branch.tcl b/git-gui/lib/branch.tcl +index 8b0c4858890f11..39e0f2dc9864e0 100644 +--- a/git-gui/lib/branch.tcl ++++ b/git-gui/lib/branch.tcl +@@ -7,7 +7,7 @@ proc load_all_heads {} { + set rh refs/heads + set rh_len [expr {[string length $rh] + 1}] + set all_heads [list] +- set fd [git_read for-each-ref --format=%(refname) $rh] ++ set fd [git_read [list for-each-ref --format=%(refname) $rh]] + fconfigure $fd -translation binary -encoding utf-8 + while {[gets $fd line] > 0} { + if {!$some_heads_tracking || ![is_tracking_branch $line]} { +@@ -21,10 +21,10 @@ proc load_all_heads {} { + + proc load_all_tags {} { + set all_tags [list] +- set fd [git_read for-each-ref \ ++ set fd [git_read [list for-each-ref \ + --sort=-taggerdate \ + --format=%(refname) \ +- refs/tags] ++ refs/tags]] + fconfigure $fd -translation binary -encoding utf-8 + while {[gets $fd line] > 0} { + if {![regsub ^refs/tags/ $line {} name]} continue +diff --git a/git-gui/lib/browser.tcl b/git-gui/lib/browser.tcl +index a98298366763d8..6fc8d4d6372264 100644 +--- a/git-gui/lib/browser.tcl ++++ b/git-gui/lib/browser.tcl +@@ -196,7 +196,7 @@ method _ls {tree_id {name {}}} { + lappend browser_stack [list $tree_id $name] + $w conf -state disabled + +- set fd [git_read ls-tree -z $tree_id] ++ set fd [git_read [list ls-tree -z $tree_id]] + fconfigure $fd -blocking 0 -translation binary -encoding utf-8 + fileevent $fd readable [cb _read $fd] + } +diff --git a/git-gui/lib/checkout_op.tcl b/git-gui/lib/checkout_op.tcl +index 31992f461bc889..48fd1a3cacdca7 100644 +--- a/git-gui/lib/checkout_op.tcl ++++ b/git-gui/lib/checkout_op.tcl +@@ -304,12 +304,12 @@ The rescan will be automatically started now. + _readtree $this + } else { + ui_status [mc "Refreshing file status..."] +- set fd [git_read update-index \ ++ set fd [git_read [list update-index \ + -q \ + --unmerged \ + --ignore-missing \ + --refresh \ +- ] ++ ]] + fconfigure $fd -blocking 0 -translation binary + fileevent $fd readable [cb _refresh_wait $fd] + } +@@ -345,7 +345,7 @@ method _readtree {} { + [mc "Updating working directory to '%s'..." [_name $this]] \ + [mc "files checked out"]] + +- set fd [git_read read-tree \ ++ set fd [git_read [list read-tree \ + -m \ + -u \ + -v \ +@@ -353,7 +353,7 @@ method _readtree {} { + $HEAD \ + $new_hash \ + 2>@1 \ +- ] ++ ]] + fconfigure $fd -blocking 0 -translation binary + fileevent $fd readable [cb _readtree_wait $fd $status_bar_operation] + } +@@ -573,7 +573,7 @@ method _confirm_reset {cur} { + pack $w.buttons.cancel -side right -padx 5 + pack $w.buttons -side bottom -fill x -pady 10 -padx 10 + +- set fd [git_read rev-list --pretty=oneline $cur ^$new_hash] ++ set fd [git_read [list rev-list --pretty=oneline $cur ^$new_hash]] + while {[gets $fd line] > 0} { + set abbr [string range $line 0 7] + set subj [string range $line 41 end] +diff --git a/git-gui/lib/choose_repository.tcl b/git-gui/lib/choose_repository.tcl +index 7bd738e51e0a9e..7b64a112395d68 100644 +--- a/git-gui/lib/choose_repository.tcl ++++ b/git-gui/lib/choose_repository.tcl +@@ -818,9 +818,9 @@ method _clone_refs {} { + error_popup [mc "Not a Git repository: %s" [file tail $origin_url]] + return 0 + } +- set fd_in [git_read for-each-ref \ ++ set fd_in [git_read [list for-each-ref \ + --tcl \ +- {--format=list %(refname) %(objectname) %(*objectname)}] ++ {--format=list %(refname) %(objectname) %(*objectname)}]] + cd $pwd + + set fd [safe_open_file [gitdir packed-refs] w] +@@ -953,14 +953,14 @@ method _do_clone_checkout {HEAD} { + [mc "files"]] + + set readtree_err {} +- set fd [git_read read-tree \ ++ set fd [git_read [list read-tree \ + -m \ + -u \ + -v \ + HEAD \ + HEAD \ + 2>@1 \ +- ] ++ ]] + fconfigure $fd -blocking 0 -translation binary + fileevent $fd readable [cb _readtree_wait $fd] + } +diff --git a/git-gui/lib/choose_rev.tcl b/git-gui/lib/choose_rev.tcl +index 7cf9e160fa3b54..8ae7e8a5c4797e 100644 +--- a/git-gui/lib/choose_rev.tcl ++++ b/git-gui/lib/choose_rev.tcl +@@ -146,14 +146,14 @@ constructor _new {path unmerged_only title} { + append fmt { %(*subject)} + append fmt {]} + set all_refn [list] +- set fr_fd [git_read for-each-ref \ ++ set fr_fd [git_read [list for-each-ref \ + --tcl \ + --sort=-taggerdate \ + --format=$fmt \ + refs/heads \ + refs/remotes \ + refs/tags \ +- ] ++ ]] + fconfigure $fr_fd -translation lf -encoding utf-8 + while {[gets $fr_fd line] > 0} { + set line [eval $line] +@@ -176,7 +176,7 @@ constructor _new {path unmerged_only title} { + close $fr_fd + + if {$unmerged_only} { +- set fr_fd [git_read rev-list --all ^$::HEAD] ++ set fr_fd [git_read [list rev-list --all ^$::HEAD]] + while {[gets $fr_fd sha1] > 0} { + if {[catch {set rlst $cmt_refn($sha1)}]} continue + foreach refn $rlst { +diff --git a/git-gui/lib/commit.tcl b/git-gui/lib/commit.tcl +index 8d135845a5ec4f..b27e37d38514e0 100644 +--- a/git-gui/lib/commit.tcl ++++ b/git-gui/lib/commit.tcl +@@ -27,7 +27,7 @@ You are currently in the middle of a merge that has not been fully completed. Y + if {[catch { + set name "" + set email "" +- set fd [git_read cat-file commit $curHEAD] ++ set fd [git_read [list cat-file commit $curHEAD]] + fconfigure $fd -encoding binary -translation lf + # By default commits are assumed to be in utf-8 + set enc utf-8 +@@ -325,7 +325,7 @@ proc commit_commitmsg_wait {fd_ph curHEAD msg_p} { + + proc commit_writetree {curHEAD msg_p} { + ui_status [mc "Committing changes..."] +- set fd_wt [git_read write-tree] ++ set fd_wt [git_read [list write-tree]] + fileevent $fd_wt readable \ + [list commit_committree $fd_wt $curHEAD $msg_p] + } +@@ -350,7 +350,7 @@ proc commit_committree {fd_wt curHEAD msg_p} { + # -- Verify this wasn't an empty change. + # + if {$commit_type eq {normal}} { +- set fd_ot [git_read cat-file commit $PARENT] ++ set fd_ot [git_read [list cat-file commit $PARENT]] + fconfigure $fd_ot -encoding binary -translation lf + set old_tree [gets $fd_ot] + close $fd_ot +diff --git a/git-gui/lib/console.tcl b/git-gui/lib/console.tcl +index c7f20b87cbf4bb..44dcdf29be8e8c 100644 +--- a/git-gui/lib/console.tcl ++++ b/git-gui/lib/console.tcl +@@ -93,7 +93,7 @@ method _init {} { + method exec {cmd {after {}}} { + lappend cmd 2>@1 + if {[lindex $cmd 0] eq {git}} { +- set fd_f [eval git_read [lrange $cmd 1 end]] ++ set fd_f [git_read [lrange $cmd 1 end]] + } else { + set fd_f [_open_stdout_stderr $cmd] + } +diff --git a/git-gui/lib/database.tcl b/git-gui/lib/database.tcl +index 85783081e0d887..1fc0ea00b3ba97 100644 +--- a/git-gui/lib/database.tcl ++++ b/git-gui/lib/database.tcl +@@ -3,7 +3,7 @@ + + proc do_stats {} { + global use_ttk NS +- set fd [git_read count-objects -v] ++ set fd [git_read [list count-objects -v]] + while {[gets $fd line] > 0} { + if {[regexp {^([^:]+): (\d+)$} $line _ name value]} { + set stats($name) $value +diff --git a/git-gui/lib/diff.tcl b/git-gui/lib/diff.tcl +index ce1bad69007bae..8ec740eb50df6c 100644 +--- a/git-gui/lib/diff.tcl ++++ b/git-gui/lib/diff.tcl +@@ -338,7 +338,7 @@ proc start_show_diff {cont_info {add_opts {}}} { + } + } + +- if {[catch {set fd [eval git_read_nice $cmd]} err]} { ++ if {[catch {set fd [git_read_nice $cmd]} err]} { + set diff_active 0 + unlock_index + ui_status [mc "Unable to display %s" [escape_path $path]] +@@ -617,7 +617,7 @@ proc apply_or_revert_hunk {x y revert} { + + if {[catch { + set enc [get_path_encoding $current_diff_path] +- set p [eval git_write $apply_cmd] ++ set p [git_write $apply_cmd] + fconfigure $p -translation binary -encoding $enc + puts -nonewline $p $wholepatch + close $p} err]} { +@@ -853,7 +853,7 @@ proc apply_or_revert_range_or_line {x y revert} { + + if {[catch { + set enc [get_path_encoding $current_diff_path] +- set p [eval git_write $apply_cmd] ++ set p [git_write $apply_cmd] + fconfigure $p -translation binary -encoding $enc + puts -nonewline $p $current_diff_header + puts -nonewline $p $wholepatch +@@ -890,7 +890,7 @@ proc undo_last_revert {} { + + if {[catch { + set enc $last_revert_enc +- set p [eval git_write $apply_cmd] ++ set p [git_write $apply_cmd] + fconfigure $p -translation binary -encoding $enc + puts -nonewline $p $last_revert + close $p} err]} { +diff --git a/git-gui/lib/index.tcl b/git-gui/lib/index.tcl +index d2ec24bd80e12a..857864ff2be2ca 100644 +--- a/git-gui/lib/index.tcl ++++ b/git-gui/lib/index.tcl +@@ -75,7 +75,7 @@ proc update_indexinfo {msg path_list after} { + if {$batch > 25} {set batch 25} + + set status_bar_operation [$::main_status start $msg [mc "files"]] +- set fd [git_write update-index -z --index-info] ++ set fd [git_write [list update-index -z --index-info]] + fconfigure $fd \ + -blocking 0 \ + -buffering full \ +@@ -144,7 +144,7 @@ proc update_index {msg path_list after} { + if {$batch > 25} {set batch 25} + + set status_bar_operation [$::main_status start $msg [mc "files"]] +- set fd [git_write update-index --add --remove -z --stdin] ++ set fd [git_write [list update-index --add --remove -z --stdin]] + fconfigure $fd \ + -blocking 0 \ + -buffering full \ +@@ -218,13 +218,13 @@ proc checkout_index {msg path_list after capture_error} { + if {$batch > 25} {set batch 25} + + set status_bar_operation [$::main_status start $msg [mc "files"]] +- set fd [git_write checkout-index \ ++ set fd [git_write [list checkout-index \ + --index \ + --quiet \ + --force \ + -z \ + --stdin \ +- ] ++ ]] + fconfigure $fd \ + -blocking 0 \ + -buffering full \ +diff --git a/git-gui/lib/merge.tcl b/git-gui/lib/merge.tcl +index e97c91eab67ae2..6317c32127b399 100644 +--- a/git-gui/lib/merge.tcl ++++ b/git-gui/lib/merge.tcl +@@ -239,7 +239,7 @@ Continue with resetting the current changes?"] + } + + if {[ask_popup $op_question] eq {yes}} { +- set fd [git_read read-tree --reset -u -v HEAD 2>@1] ++ set fd [git_read [list read-tree --reset -u -v HEAD 2>@1]] + fconfigure $fd -blocking 0 -translation binary + set status_bar_operation [$::main_status \ + start \ +diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl +index f2fa4393057d66..777d7b323bcd26 100644 +--- a/git-gui/lib/mergetool.tcl ++++ b/git-gui/lib/mergetool.tcl +@@ -88,7 +88,7 @@ proc merge_load_stages {path cont} { + set merge_stages(3) {} + set merge_stages_buf {} + +- set merge_stages_fd [eval git_read ls-files -u -z -- {$path}] ++ set merge_stages_fd [git_read [list ls-files -u -z -- $path]] + + fconfigure $merge_stages_fd -blocking 0 -translation binary -encoding binary + fileevent $merge_stages_fd readable [list read_merge_stages $merge_stages_fd $cont] +diff --git a/git-gui/lib/remote.tcl b/git-gui/lib/remote.tcl +index a733a0f36e6e21..cf796d1601437f 100644 +--- a/git-gui/lib/remote.tcl ++++ b/git-gui/lib/remote.tcl +@@ -32,7 +32,7 @@ proc all_tracking_branches {} { + } + + if {$pat ne {}} { +- set fd [eval git_read for-each-ref --format=%(refname) $cmd] ++ set fd [git_read [concat for-each-ref --format=%(refname) $cmd]] + while {[gets $fd n] > 0} { + foreach spec $pat { + set dst [string range [lindex $spec 0] 0 end-2] +diff --git a/git-gui/lib/remote_branch_delete.tcl b/git-gui/lib/remote_branch_delete.tcl +index 5ba9fcadd17f31..c8c99b17a8da39 100644 +--- a/git-gui/lib/remote_branch_delete.tcl ++++ b/git-gui/lib/remote_branch_delete.tcl +@@ -308,7 +308,7 @@ method _load {cache uri} { + set full_list [list] + set head_cache($cache) [list] + set full_cache($cache) [list] +- set active_ls [git_read ls-remote $uri] ++ set active_ls [git_read [list ls-remote $uri]] + fconfigure $active_ls \ + -blocking 0 \ + -translation lf \ + +From 1e0a93c3d35c84547b21ba704a9c4383d4360140 Mon Sep 17 00:00:00 2001 +From: Johannes Sixt +Date: Sun, 4 May 2025 15:06:11 +0200 +Subject: [PATCH 37/41] git-gui: pass redirections as separate argument to + _open_stdout_stderr + +We are going to treat command arguments and redirections differently to +avoid passing arguments that look like redirections to the command +accidentally. To do so, it will be necessary to know which arguments +are intentional redirections. Rewrite direct callers of +_open_stdout_stderr to pass intentional redirections as a second +(optional) argument. + +Passing arbitrary arguments is not safe right now, but we rename it +to safe_open_command anyway to avoid having to touch the call sites +again later when we make it actually safe. + +We cannot make the function safe right away because one caller is +git_read, which does not yet know which of its arguments are +redirections. This is the topic of the next commit. + +Signed-off-by: Johannes Sixt + +Signed-off-by: Taylor Blau +--- + git-gui/git-gui.sh | 10 +++++----- + git-gui/lib/console.tcl | 4 ++-- + git-gui/lib/mergetool.tcl | 4 ++-- + git-gui/lib/sshkey.tcl | 2 +- + git-gui/lib/tools.tcl | 3 +-- + 5 files changed, 11 insertions(+), 12 deletions(-) + +diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh +index 301c7647ecaec6..408149b5309f68 100755 +--- a/git-gui/git-gui.sh ++++ b/git-gui/git-gui.sh +@@ -631,10 +631,10 @@ proc git {args} { + return $result + } + +-proc _open_stdout_stderr {cmd} { +- _trace_exec $cmd ++proc safe_open_command {cmd {redir {}}} { ++ _trace_exec [concat $cmd $redir] + if {[catch { +- set fd [open [concat [list | ] $cmd] r] ++ set fd [open [concat [list | ] $cmd $redir] r] + } err]} { + error $err + } +@@ -646,7 +646,7 @@ proc git_read {cmd} { + set cmdp [_git_cmd [lindex $cmd 0]] + set cmd [lrange $cmd 1 end] + +- return [_open_stdout_stderr [concat $cmdp $cmd]] ++ return [safe_open_command [concat $cmdp $cmd]] + } + + proc git_read_nice {cmd} { +@@ -657,7 +657,7 @@ proc git_read_nice {cmd} { + set cmdp [_git_cmd [lindex $cmd 0]] + set cmd [lrange $cmd 1 end] + +- return [_open_stdout_stderr [concat $opt $cmdp $cmd]] ++ return [safe_open_command [concat $opt $cmdp $cmd]] + } + + proc git_write {cmd} { +diff --git a/git-gui/lib/console.tcl b/git-gui/lib/console.tcl +index 44dcdf29be8e8c..cc416d48119cf3 100644 +--- a/git-gui/lib/console.tcl ++++ b/git-gui/lib/console.tcl +@@ -91,11 +91,11 @@ method _init {} { + } + + method exec {cmd {after {}}} { +- lappend cmd 2>@1 + if {[lindex $cmd 0] eq {git}} { ++ lappend cmd 2>@1 + set fd_f [git_read [lrange $cmd 1 end]] + } else { +- set fd_f [_open_stdout_stderr $cmd] ++ set fd_f [safe_open_command $cmd [list 2>@1]] + } + fconfigure $fd_f -blocking 0 -translation binary + fileevent $fd_f readable [cb _read $fd_f $after] +diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl +index 777d7b323bcd26..6b267264185aaa 100644 +--- a/git-gui/lib/mergetool.tcl ++++ b/git-gui/lib/mergetool.tcl +@@ -343,9 +343,9 @@ proc merge_tool_start {cmdline target backup stages} { + + # Force redirection to avoid interpreting output on stderr + # as an error, and launch the tool +- lappend cmdline {2>@1} ++ set redir [list {2>@1}] + +- if {[catch { set mtool_fd [_open_stdout_stderr $cmdline] } err]} { ++ if {[catch { set mtool_fd [safe_open_command $cmdline $redir] } err]} { + delete_temp_files $mtool_tmpfiles + error_popup [mc "Could not start the merge tool:\n\n%s" $err] + return +diff --git a/git-gui/lib/sshkey.tcl b/git-gui/lib/sshkey.tcl +index 2e006cb8ca6706..b32bdd06e95d70 100644 +--- a/git-gui/lib/sshkey.tcl ++++ b/git-gui/lib/sshkey.tcl +@@ -85,7 +85,7 @@ proc make_ssh_key {w} { + set cmdline [list [shellpath] -c \ + {echo | ssh-keygen -q -t rsa -f ~/.ssh/id_rsa 2>&1}] + +- if {[catch { set sshkey_fd [_open_stdout_stderr $cmdline] } err]} { ++ if {[catch { set sshkey_fd [safe_open_command $cmdline] } err]} { + error_popup [mc "Could not start ssh-keygen:\n\n%s" $err] + return + } +diff --git a/git-gui/lib/tools.tcl b/git-gui/lib/tools.tcl +index 413f1a170079e0..142ffceeddcf73 100644 +--- a/git-gui/lib/tools.tcl ++++ b/git-gui/lib/tools.tcl +@@ -130,8 +130,7 @@ proc tools_exec {fullname} { + } + + proc tools_run_silent {cmd after} { +- lappend cmd 2>@1 +- set fd [_open_stdout_stderr $cmd] ++ set fd [safe_open_command $cmd [list 2>@1]] + + fconfigure $fd -blocking 0 -translation binary + fileevent $fd readable [list tools_consume_input $fd $after] + +From 60b0ba0a04c413b716dc33f83285ede26e820668 Mon Sep 17 00:00:00 2001 +From: Johannes Sixt +Date: Sun, 4 May 2025 15:39:03 +0200 +Subject: [PATCH 38/41] git-gui: pass redirections as separate argument to + git_read + +We are going to treat command arguments and redirections differently to +avoid passing arguments that look like redirections to the command +accidentally. To do so, it will be necessary to know which arguments +are intentional redirections. Rewrite direct call sites of git_read +to pass intentional redirections as a second (optional) argument. + +git_read defers to safe_open_command, but we cannot make it safe, yet, +because one of the callers of git_read is proc git, which does not yet +know which of its arguments are redirections. This is the topic of the +next commit. + +Signed-off-by: Johannes Sixt +Signed-off-by: Taylor Blau +--- + git-gui/git-gui.sh | 6 +++--- + git-gui/lib/checkout_op.tcl | 4 ++-- + git-gui/lib/choose_repository.tcl | 4 ++-- + git-gui/lib/console.tcl | 3 +-- + git-gui/lib/merge.tcl | 2 +- + 5 files changed, 9 insertions(+), 10 deletions(-) + +diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh +index 408149b5309f68..bbdbd35d269507 100755 +--- a/git-gui/git-gui.sh ++++ b/git-gui/git-gui.sh +@@ -642,11 +642,11 @@ proc safe_open_command {cmd {redir {}}} { + return $fd + } + +-proc git_read {cmd} { ++proc git_read {cmd {redir {}}} { + set cmdp [_git_cmd [lindex $cmd 0]] + set cmd [lrange $cmd 1 end] + +- return [safe_open_command [concat $cmdp $cmd]] ++ return [safe_open_command [concat $cmdp $cmd] $redir] + } + + proc git_read_nice {cmd} { +diff --git a/git-gui/lib/checkout_op.tcl b/git-gui/lib/checkout_op.tcl +index 48fd1a3cacdca7..87ed0b48585e05 100644 +--- a/git-gui/lib/checkout_op.tcl ++++ b/git-gui/lib/checkout_op.tcl +@@ -352,8 +352,8 @@ method _readtree {} { + --exclude-per-directory=.gitignore \ + $HEAD \ + $new_hash \ +- 2>@1 \ +- ]] ++ ] \ ++ [list 2>@1]] + fconfigure $fd -blocking 0 -translation binary + fileevent $fd readable [cb _readtree_wait $fd $status_bar_operation] + } +diff --git a/git-gui/lib/choose_repository.tcl b/git-gui/lib/choose_repository.tcl +index 7b64a112395d68..5b361cc424a6f6 100644 +--- a/git-gui/lib/choose_repository.tcl ++++ b/git-gui/lib/choose_repository.tcl +@@ -959,8 +959,8 @@ method _do_clone_checkout {HEAD} { + -v \ + HEAD \ + HEAD \ +- 2>@1 \ +- ]] ++ ] \ ++ [list 2>@1]] + fconfigure $fd -blocking 0 -translation binary + fileevent $fd readable [cb _readtree_wait $fd] + } +diff --git a/git-gui/lib/console.tcl b/git-gui/lib/console.tcl +index cc416d48119cf3..4715ce91e67887 100644 +--- a/git-gui/lib/console.tcl ++++ b/git-gui/lib/console.tcl +@@ -92,8 +92,7 @@ method _init {} { + + method exec {cmd {after {}}} { + if {[lindex $cmd 0] eq {git}} { +- lappend cmd 2>@1 +- set fd_f [git_read [lrange $cmd 1 end]] ++ set fd_f [git_read [lrange $cmd 1 end] [list 2>@1]] + } else { + set fd_f [safe_open_command $cmd [list 2>@1]] + } +diff --git a/git-gui/lib/merge.tcl b/git-gui/lib/merge.tcl +index 6317c32127b399..55b4fc5ed3736d 100644 +--- a/git-gui/lib/merge.tcl ++++ b/git-gui/lib/merge.tcl +@@ -239,7 +239,7 @@ Continue with resetting the current changes?"] + } + + if {[ask_popup $op_question] eq {yes}} { +- set fd [git_read [list read-tree --reset -u -v HEAD 2>@1]] ++ set fd [git_read [list read-tree --reset -u -v HEAD] [list 2>@1]] + fconfigure $fd -blocking 0 -translation binary + set status_bar_operation [$::main_status \ + start \ + +From 99f7bc1af65fabab907bf35e645241f714e7386e Mon Sep 17 00:00:00 2001 +From: Johannes Sixt +Date: Sun, 4 May 2025 20:26:11 +0200 +Subject: [PATCH 39/41] git-gui: introduce function git_redir for git calls + with redirections + +Proc git invokes git and collects all output, which is it returns. +We are going to treat command arguments and redirections differently to +avoid passing arguments that look like redirections to the command +accidentally. A few invocations also pass redirection operators as +command arguments deliberately. Rewrite these cases to use a new +function git_redir that takes two lists, one for the regular command +arguments and one for the redirection operations. + +Signed-off-by: Johannes Sixt + +Signed-off-by: Taylor Blau +--- + git-gui/git-gui.sh | 8 ++++++-- + git-gui/lib/commit.tcl | 4 ++-- + git-gui/lib/merge.tcl | 2 +- + 3 files changed, 9 insertions(+), 5 deletions(-) + +diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh +index bbdbd35d269507..75d7b9b9fc81bc 100755 +--- a/git-gui/git-gui.sh ++++ b/git-gui/git-gui.sh +@@ -621,7 +621,11 @@ proc _lappend_nice {cmd_var} { + } + + proc git {args} { +- set fd [git_read $args] ++ git_redir $args {} ++} ++ ++proc git_redir {cmd redir} { ++ set fd [git_read $cmd $redir] + fconfigure $fd -translation binary -encoding utf-8 + set result [string trimright [read $fd] "\n"] + close $fd +@@ -1423,7 +1427,7 @@ proc PARENT {} { + return $p + } + if {$empty_tree eq {}} { +- set empty_tree [git mktree << {}] ++ set empty_tree [git_redir [list mktree] [list << {}]] + } + return $empty_tree + } +diff --git a/git-gui/lib/commit.tcl b/git-gui/lib/commit.tcl +index b27e37d38514e0..bb6056d0adb992 100644 +--- a/git-gui/lib/commit.tcl ++++ b/git-gui/lib/commit.tcl +@@ -388,8 +388,8 @@ A rescan will be automatically started now. + foreach p [concat $PARENT $MERGE_HEAD] { + lappend cmd -p $p + } +- lappend cmd <$msg_p +- if {[catch {set cmt_id [eval git $cmd]} err]} { ++ set msgtxt [list <$msg_p] ++ if {[catch {set cmt_id [git_redir $cmd $msgtxt]} err]} { + catch {file delete $msg_p} + error_popup [strcat [mc "commit-tree failed:"] "\n\n$err"] + ui_status [mc "Commit failed."] +diff --git a/git-gui/lib/merge.tcl b/git-gui/lib/merge.tcl +index 55b4fc5ed3736d..44c3f93584224c 100644 +--- a/git-gui/lib/merge.tcl ++++ b/git-gui/lib/merge.tcl +@@ -118,7 +118,7 @@ method _start {} { + set cmd [list git] + lappend cmd merge + lappend cmd --strategy=recursive +- lappend cmd [git fmt-merge-msg <[gitdir FETCH_HEAD]] ++ lappend cmd [git_redir [list fmt-merge-msg] [list <[gitdir FETCH_HEAD]]] + lappend cmd HEAD + lappend cmd $name + } + +From 44e3935d53e3c0b00ff35bea4fcf8e1731ee4f9b Mon Sep 17 00:00:00 2001 +From: Johannes Sixt +Date: Sun, 4 May 2025 21:59:19 +0200 +Subject: [PATCH 40/41] git-gui: do not mistake command arguments as + redirection operators + +Tcl 'open' assigns special meaning to its argument when they begin with +redirection, pipe or background operator. There are many calls of the +'open' variant that runs a process which construct arguments that are +taken from the Git repository or are 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 'open' lest it redirects input or output, or attempts to build a +pipeline using a command name controlled by the repository. + +Use the helper function make_arglist_safe, which identifies such +arguments and prepends "./" to force such a name to be regarded as a +relative file name. + +After this change the following 'open' calls that start a process do not +apply the argument processing: + +git-gui/git-gui.sh:4095: || [catch {set spell_fd [open $spell_cmd r+]} spell_err]} { +lib/spellcheck.tcl:47: set pipe_fd [open [list | $s_prog -v] r] +lib/spellcheck.tcl:133: _connect $this [open $spell_cmd r+] +lib/spellcheck.tcl:405: set fd [open [list | aspell dump dicts] r] + +In all cases, the command arguments are constant strings (or begin with +a constant string) that are of a form that would not be affected by the +processing anyway. + +Signed-off-by: Johannes Sixt + +Signed-off-by: Taylor Blau +--- + git-gui/git-gui.sh | 3 +++ + 1 file changed, 3 insertions(+) + +diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh +index 75d7b9b9fc81bc..9ad172ac6602e2 100755 +--- a/git-gui/git-gui.sh ++++ b/git-gui/git-gui.sh +@@ -600,6 +600,7 @@ proc _git_cmd {name} { + # how to run. + proc open_cmd_pipe {cmd path} { + set run [list [shellpath] -c "$cmd \"\$0\"" $path] ++ set run [make_arglist_safe $run] + return [open |$run r] + } + +@@ -636,6 +637,7 @@ proc git_redir {cmd redir} { + } + + proc safe_open_command {cmd {redir {}}} { ++ set cmd [make_arglist_safe $cmd] + _trace_exec [concat $cmd $redir] + if {[catch { + set fd [open [concat [list | ] $cmd $redir] r] +@@ -665,6 +667,7 @@ proc git_read_nice {cmd} { + } + + proc git_write {cmd} { ++ set cmd [make_arglist_safe $cmd] + set cmdp [_git_cmd [lindex $cmd 0]] + set cmd [lrange $cmd 1 end] + + +From a437f5bc93330a70b42a230e52f3bd036ca1b1da Mon Sep 17 00:00:00 2001 +From: Johannes Sixt +Date: Wed, 14 May 2025 21:06:37 +0200 +Subject: [PATCH 41/41] git-gui: sanitize 'exec' arguments: convert new + 'cygpath' calls + +The side branch merged in the previous commit introduces new 'exec' +calls. Convert these in the same way we did earlier for existing +'exec' calls. + +Signed-off-by: Johannes Sixt + +Signed-off-by: Taylor Blau +--- + git-gui/git-gui.sh | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh +index 0355c0c8365047..2c446c27845c16 100755 +--- a/git-gui/git-gui.sh ++++ b/git-gui/git-gui.sh +@@ -393,7 +393,7 @@ if {[string match @@* $_shellpath]} { + } + + if {[is_Windows]} { +- set _shellpath [exec cygpath -m $_shellpath] ++ set _shellpath [safe_exec [list cygpath -m $_shellpath]] + } + + if {![file executable $_shellpath] || \ +@@ -2778,7 +2778,7 @@ if {![is_bare]} { + + if {[is_Windows]} { + # Use /git-bash.exe if available +- set _git_bash [exec cygpath -m /git-bash.exe] ++ set _git_bash [safe_exec [list cygpath -m /git-bash.exe]] + if {[file executable $_git_bash]} { + set _bash_cmdline [list "Git Bash" $_git_bash] + } else { diff --git a/git.spec b/git.spec index 7bbd6894fae5bd9057b1eb1b4dacf16f388b0fc0..3041a2eec8b9e1a4d833b4c005a4a59d9e026d32 100644 --- a/git.spec +++ b/git.spec @@ -1,7 +1,7 @@ %global gitexecdir %{_libexecdir}/git-core Name: git Version: 2.33.0 -Release: 18 +Release: 19 Summary: A popular and widely used Version Control System License: GPLv2+ or LGPLv2.1 URL: https://git-scm.com/ @@ -77,6 +77,7 @@ Patch61: backport-CVE-2024-52006-credential-disallow-Carriage-Returns-in-the- Patch62: backport-CVE-2024-52005.patch Patch63: backport-CVE-2025-48384-config-quote-values-containing-CR-character.patch Patch64: backport-CVE-2025-48386-wincred-avoid-buffer-overflow-in-wcsncat.patch +Patch65: backport-CVE-2025-27613-CVE-2025-46334-CVE-2025-46835.patch BuildRequires: gcc gettext BuildRequires: openssl-devel libcurl-devel expat-devel systemd asciidoc xmlto glib2-devel libsecret-devel pcre2-devel desktop-file-utils @@ -360,6 +361,12 @@ make %{?_smp_mflags} test %{_mandir}/man7/git*.7.* %changelog +* Mon Jul 14 2025 fuanan - 2.33.0-19 +- Type:CVE +- ID:CVE-2025-27613 CVE-2025-46334 CVE-2025-46835 +- SUG:NA +- DESC:Fix CVE-2025-27613 CVE-2025-46334 CVE-2025-46835 + * Wed Jul 9 2025 fuanan - 2.33.0-18 - Type:CVE - ID:CVE-2025-48384 CVE-2025-48386