From b966b738e1923badc788b9111cc81653b50ff164 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Mon, 17 Mar 2025 20:36:04 +0100 Subject: [PATCH 01/51] 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 b/gitk index 0ae7d68590..ffb5382b49 100755 --- a/gitk +++ b/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/51] 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 b/gitk index ffb5382b49..b061377c7b 100755 --- a/gitk +++ b/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/51] 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 b/gitk index b061377c7b..5b844730b1 100755 --- a/gitk +++ b/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/51] 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 b/gitk index 5b844730b1..d748d19d90 100755 --- a/gitk +++ b/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/51] 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 b/gitk index d748d19d90..218f61fa28 100755 --- a/gitk +++ b/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/51] 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 b/gitk index 218f61fa28..c0d793f05d 100755 --- a/gitk +++ b/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/51] 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 b/gitk index c0d793f05d..9673e56abd 100755 --- a/gitk +++ b/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/51] 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 b/gitk index 9673e56abd..aba8ef63dc 100755 --- a/gitk +++ b/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 @@ -463,8 +470,8 @@ proc start_rev_list {view} { } if {[catch { - set fd [open [concat | git log --no-color -z --pretty=raw $show_notes \ - --parents --boundary $args "--" $files] r] + set fd [safe_open_command [concat git log --no-color -z --pretty=raw $show_notes \ + --parents --boundary $args "--" $files]] } err]} { error_popup "[mc "Error executing git log:"] $err" return 0 @@ -611,8 +618,8 @@ proc updatecommits {} { set args $vorigargs($view) } if {[catch { - set fd [open [concat | git log --no-color -z --pretty=raw $show_notes \ - --parents --boundary $args "--" $vfilelimit($view)] r] + set fd [safe_open_command [concat git log --no-color -z --pretty=raw $show_notes \ + --parents --boundary $args "--" $vfilelimit($view)]] } err]} { error_popup "[mc "Error executing git log:"] $err" return @@ -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" @@ -10281,7 +10288,7 @@ proc getallcommits {} { } } if {$ids ne {}} { - set fd [open [concat $cmd $ids] r] + set fd [safe_open_command [concat $cmd $ids]] fconfigure $fd -blocking 0 incr allcommits nowbusy allcommits From 42a64b41a7a3d01a62f0f34f75bee2bbd00be46f Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Thu, 20 Mar 2025 20:00:57 +0100 Subject: [PATCH 09/51] 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 b/gitk index aba8ef63dc..68d6bfd61f 100755 --- a/gitk +++ b/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/51] 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 b/gitk index 68d6bfd61f..22da6a811c 100755 --- a/gitk +++ b/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/51] 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 b/gitk index 22da6a811c..2e37ddea96 100755 --- a/gitk +++ b/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/51] 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 b/gitk index 2e37ddea96..9bd226ec83 100755 --- a/gitk +++ b/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 8e3070aa5e331be45d4d03e3be41f84494fce129 Mon Sep 17 00:00:00 2001 From: "Avi Halachmi (:avih)" Date: Fri, 7 Mar 2025 13:48:57 +0200 Subject: [PATCH 13/51] gitk: encode arguments correctly with "open" While "exec" uses a normal arguments list which is applied as command + arguments (and redirections, etc), "open" uses a single argument which is this command+arguments, where the command and arguments are a list inside this one argument to "open". Commit bb5cb23 (gitk: prevent overly long command lines 2023-05-08) changed several values from individual arguments in that list (hashes and file names), to a single value which is fed to git via redirection to its stdin using "open" [1]. However, it didn't ensure correctly that this aggregate value in this string is interpreted as a single element in this command+args list. It did just enough so that newlines (which is how these elements are concatenated) don't split this single list element. A followup commit at the same patchset: 7dd272e (gitk: escape file paths before piping to git log 2023-05-08) added a bit more, by escaping backslahes and spaces at the file names, so that at least it doesn't break when such file names get used there. But these are not enough. At the very least tab is missing, and more, and trying to manually escape every possible thing which can affect how this string is interpreted in a list is a sub-par approach. The solution is simply to tell tcl "this is a single list element". which we can do by aggregating this value completely normally (hashes and files separated by newlines), and then do [list $value]. So this is what this commit does, for all 3 places where bb5cb23 changed individual elements into an aggregate value. [1] That was not a fully accurate description. The accurate version is that this string originally included two lists: hashes and files. When used with "open" these lists correctly become the individual elements of these lists, even if they contain spaces etc, so the arguments which were used at this "git" commands were correct. Commit bb5cb23 couldn't use these two lists as-is, because it needed to process the individual elements in them (one element per line of the aggregate value), and the issue is that ensuring this aggregate is indeed interpreted as a single list element was sub-par. Note: all the (double) quotes before/after the modification are not required and with zero effect, even for \n. But this commit preserves the original quoting form intentionally. It can be cleaned up later. Signed-off-by: Avi Halachmi (:avih) Signed-off-by: Johannes Sixt Signed-off-by: Taylor Blau --- gitk | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/gitk b/gitk index df3ba2ea99..1f669c5190 100755 --- a/gitk +++ b/gitk @@ -353,16 +353,6 @@ proc parseviewrevs {view revs} { return $ret } -# Escapes a list of filter paths to be passed to git log via stdin. Note that -# paths must not be quoted. -proc escape_filter_paths {paths} { - set escaped [list] - foreach path $paths { - lappend escaped [string map {\\ \\\\ "\ " "\\\ "} $path] - } - return $escaped -} - # Start off a git log process and arrange to read its output proc start_rev_list {view} { global startmsecs commitidx viewcomplete curview @@ -424,8 +414,7 @@ proc start_rev_list {view} { if {[catch { set fd [open [concat | git log --no-color -z --pretty=raw $show_notes \ --parents --boundary $args --stdin \ - "<<[join [concat $revs "--" \ - [escape_filter_paths $files]] "\\n"]"] r] + [list "<<[join [concat $revs "--" $files] "\n"]"]] r] } err]} { error_popup "[mc "Error executing git log:"] $err" return 0 @@ -578,9 +567,7 @@ proc updatecommits {} { if {[catch { set fd [open [concat | git log --no-color -z --pretty=raw $show_notes \ --parents --boundary $args --stdin \ - "<<[join [concat $revs "--" \ - [escape_filter_paths \ - $vfilelimit($view)]] "\\n"]"] r] + [list "<<[join [concat $revs "--" $vfilelimit($view)] "\n"]"]] r] } err]} { error_popup "[mc "Error executing git log:"] $err" return @@ -10258,7 +10245,7 @@ proc getallcommits {} { if {$ids eq "--all"} { set cmd [concat $cmd "--all"] } else { - set cmd [concat $cmd --stdin "<<[join $ids "\\n"]"] + set cmd [concat $cmd --stdin [list "<<[join $ids "\n"]"]] } set fd [open $cmd r] fconfigure $fd -blocking 0 From 37b9230226db2a3a81df2e92a44ea655076cd0c4 Mon Sep 17 00:00:00 2001 From: Mark Levedahl Date: Thu, 3 Apr 2025 10:26:21 -0400 Subject: [PATCH 14/51] 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.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/git-gui.sh b/git-gui.sh index 3e5907a460..2e079b6ab1 100755 --- a/git-gui.sh +++ b/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/51] 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.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-gui.sh b/git-gui.sh index 2e079b6ab1..3135116169 100755 --- a/git-gui.sh +++ b/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/51] 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.sh | 38 ++++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index 3135116169..d56610c892 100755 --- a/git-gui.sh +++ b/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/51] 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.sh | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index 3e5907a460..ca1362aa19 100755 --- a/git-gui.sh +++ b/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/51] 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 --- lib/sshkey.tcl | 3 ++- lib/tools.tcl | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/sshkey.tcl b/lib/sshkey.tcl index 589ff8f78a..c0c5d1dad8 100644 --- a/lib/sshkey.tcl +++ b/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/lib/tools.tcl b/lib/tools.tcl index 413f1a1700..b86f72ed16 100644 --- a/lib/tools.tcl +++ b/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/51] 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 --- lib/checkout_op.tcl | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/lib/checkout_op.tcl b/lib/checkout_op.tcl index 21ea768d80..5f7011078a 100644 --- a/lib/checkout_op.tcl +++ b/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/51] 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.sh | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index d56610c892..cb0c02e5b8 100755 --- a/git-gui.sh +++ b/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/51] 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.sh | 67 ++++++++++++++++++------------------------------------ 1 file changed, 22 insertions(+), 45 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index ca1362aa19..2e325b042a 100755 --- a/git-gui.sh +++ b/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) {} - } - } +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 } - 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) {} - } + 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/51] 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.sh | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index cb0c02e5b8..72da2443e5 100755 --- a/git-gui.sh +++ b/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/51] 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.sh | 36 ++++++++++++++++++++++++------------ lib/blame.tcl | 2 +- lib/choose_repository.tcl | 14 +++++++------- lib/choose_rev.tcl | 2 +- lib/commit.tcl | 4 ++-- lib/diff.tcl | 2 +- lib/merge.tcl | 2 +- lib/mergetool.tcl | 2 +- lib/remote.tcl | 6 +++--- lib/shortcut.tcl | 4 ++-- lib/sshkey.tcl | 2 +- 11 files changed, 44 insertions(+), 32 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index 2e325b042a..52b463c45f 100755 --- a/git-gui.sh +++ b/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 @@ -527,7 +539,7 @@ proc _git_cmd {name} { # 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] + set f [safe_open_file $filename r] fconfigure $f -encoding binary set magic [read $f 2] 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/lib/blame.tcl b/lib/blame.tcl index 8441e109be..e70a079fa7 100644 --- a/lib/blame.tcl +++ b/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/lib/choose_repository.tcl b/lib/choose_repository.tcl index d23abedcb3..ef7ad7c175 100644 --- a/lib/choose_repository.tcl +++ b/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/lib/choose_rev.tcl b/lib/choose_rev.tcl index 6dae7937d5..7cf9e160fa 100644 --- a/lib/choose_rev.tcl +++ b/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/lib/commit.tcl b/lib/commit.tcl index 11379f8ad3..8d135845a5 100644 --- a/lib/commit.tcl +++ b/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/lib/diff.tcl b/lib/diff.tcl index 871ad488c2..f089fdc46b 100644 --- a/lib/diff.tcl +++ b/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/lib/merge.tcl b/lib/merge.tcl index 664803cf3f..897dc2a286 100644 --- a/lib/merge.tcl +++ b/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/lib/mergetool.tcl b/lib/mergetool.tcl index e688b016ef..f2fa439305 100644 --- a/lib/mergetool.tcl +++ b/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/lib/remote.tcl b/lib/remote.tcl index ef77ed7399..a733a0f36e 100644 --- a/lib/remote.tcl +++ b/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/lib/shortcut.tcl b/lib/shortcut.tcl index 674a41f5e0..9be79b2e89 100644 --- a/lib/shortcut.tcl +++ b/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/lib/sshkey.tcl b/lib/sshkey.tcl index 589ff8f78a..2e006cb8ca 100644 --- a/lib/sshkey.tcl +++ b/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/51] 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.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-gui.sh b/git-gui.sh index 72da2443e5..3bfe4364c7 100755 --- a/git-gui.sh +++ b/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/51] 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.sh | 42 ++++++++++++++++++++++++++++++++++++------ lib/diff.tcl | 2 +- lib/shortcut.tcl | 8 ++++---- lib/win32.tcl | 9 +++++---- 4 files changed, 46 insertions(+), 15 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index 52b463c45f..a6be30e295 100755 --- a/git-gui.sh +++ b/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/lib/diff.tcl b/lib/diff.tcl index f089fdc46b..cfa8a414d3 100644 --- a/lib/diff.tcl +++ b/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/lib/shortcut.tcl b/lib/shortcut.tcl index 9be79b2e89..d437ea6933 100644 --- a/lib/shortcut.tcl +++ b/lib/shortcut.tcl @@ -30,8 +30,8 @@ proc do_cygwin_shortcut {} { global argv0 _gitworktree oguilib if {[catch { - set desktop [exec cygpath \ - --desktop] + set desktop [safe_exec [list cygpath \ + --desktop]] }]} { set desktop . } @@ -50,14 +50,14 @@ proc do_cygwin_shortcut {} { "CHERE_INVOKING=1 \ source /etc/profile; \ git gui"} - exec /bin/mkshortcut.exe \ + safe_exec [list /bin/mkshortcut.exe \ --arguments $shargs \ --desc "git-gui on $repodir" \ --icon $oguilib/git-gui.ico \ --name $fn \ --show min \ --workingdir $repodir \ - /bin/sh.exe + /bin/sh.exe] } err]} { error_popup [strcat [mc "Cannot write shortcut:"] "\n\n$err"] } diff --git a/lib/win32.tcl b/lib/win32.tcl index db91ab84a5..3aedae2f13 100644 --- a/lib/win32.tcl +++ b/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/51] 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 --- lib/shortcut.tcl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/shortcut.tcl b/lib/shortcut.tcl index 674a41f5e0..263f4899c9 100644 --- a/lib/shortcut.tcl +++ b/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/51] 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.sh | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index a6be30e295..6ecddfdd0a 100755 --- a/git-gui.sh +++ b/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 @@ -2761,13 +2771,13 @@ if {[is_Windows]} { 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 cmdLine [list "Git Bash" $cmdLine] } else { - set cmdLine [list "Git Bash" bash --login -l &] + set cmdLine [list "Git Bash" bash --login -l] } .mbar.repository add command \ -label [mc "Git Bash"] \ - -command {eval exec [auto_execok start] $cmdLine} + -command {safe_exec_bg [concat [list [auto_execok start]] $cmdLine]} } if {[is_Windows] || ![is_bare]} { From 676c49583f063c6cb92e5b38eb6576929cebf1ff Mon Sep 17 00:00:00 2001 From: Mark Levedahl Date: Mon, 7 Apr 2025 17:12:56 -0400 Subject: [PATCH 28/51] 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.sh | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index 3bfe4364c7..570c236f57 100755 --- a/git-gui.sh +++ b/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 {eval exec [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/51] 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 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.sh | 4 ---- lib/checkout_op.tcl | 3 ++- lib/choose_repository.tcl | 3 ++- lib/console.tcl | 4 ++-- lib/merge.tcl | 2 +- 5 files changed, 7 insertions(+), 9 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index 6ecddfdd0a..890a172fc4 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -651,10 +651,6 @@ proc git_read {args} { _lappend_nice opt } - --stderr { - lappend args 2>@1 - } - default { break } diff --git a/lib/checkout_op.tcl b/lib/checkout_op.tcl index 5f7011078a..31992f461b 100644 --- a/lib/checkout_op.tcl +++ b/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/lib/choose_repository.tcl b/lib/choose_repository.tcl index ef7ad7c175..7bd738e51e 100644 --- a/lib/choose_repository.tcl +++ b/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/lib/console.tcl b/lib/console.tcl index bb6b9c889e..c7f20b87cb 100644 --- a/lib/console.tcl +++ b/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/lib/merge.tcl b/lib/merge.tcl index 897dc2a286..e97c91eab6 100644 --- a/lib/merge.tcl +++ b/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/51] 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.sh | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index 570c236f57..9ccd888930 100755 --- a/git-gui.sh +++ b/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/51] 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.sh | 43 ++++++++++--------------------------------- lib/blame.tcl | 2 +- lib/diff.tcl | 2 +- 3 files changed, 12 insertions(+), 35 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index 890a172fc4..28113220af 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -643,22 +643,16 @@ proc _open_stdout_stderr {cmd} { } proc git_read {args} { + set cmdp [_git_cmd [lindex $args 0]] + set args [lrange $args 1 end] + + return [_open_stdout_stderr [concat $cmdp $args]] +} + +proc git_read_nice {args} { set opt [list] - while {1} { - switch -- [lindex $args 0] { - --nice { - _lappend_nice opt - } - - default { - break - } - - } - - 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/lib/blame.tcl b/lib/blame.tcl index e70a079fa7..ceb2330266 100644 --- a/lib/blame.tcl +++ b/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/lib/diff.tcl b/lib/diff.tcl index cfa8a414d3..ce1bad6900 100644 --- a/lib/diff.tcl +++ b/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/51] 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.sh | 64 +++++++++++++++++++++++++++++------------------------- 1 file changed, 34 insertions(+), 30 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index 9ccd888930..217aeb9ce3 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -77,39 +77,43 @@ proc is_Cygwin {} { ###################################################################### ## -## PATH lookup +## PATH lookup. Sanitize $PATH, assure exec/open use only that + +if {[is_Windows]} { + set _path_sep {;} + set _search_exe .exe +} else { + set _path_sep {:} + 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 {} -proc _which {what args} { - global env _search_exe _search_path - - if {$_search_path eq {}} { - if {[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 {} - } +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 074c2b9d7c4b1201f261263f011074c733a85d38 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Sat, 3 May 2025 19:21:53 +0200 Subject: [PATCH 33/51] git-gui: use git_read in githook_read 0730a5a3a5e6 ("git-gui - use git-hook, honor core.hooksPath", 2023-09-17) rewrote githook_read to use `git hook` to run a hook script. The code that was replaced discovered the hook script file manually and invoked it using function _open_stdout_stderr. After the rewrite, this function is still invoked, but it calls into `git` instead of the hook scripts. Notice though, that we have function git_read that invokes git and prepares a pipe for the caller to read from. Replace the implementation of githook_read to be just a wrapper around git_read. This unifies the way in which the git executable is invoked. git_read ultimately also calls into _open_stdout_stderr, but it modifies the path to the git executable before doing so. Signed-off-by: Johannes Sixt Signed-off-by: Taylor Blau --- git-gui.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index 28113220af..e10bf2a039 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -669,8 +669,7 @@ proc git_write {args} { } proc githook_read {hook_name args} { - set cmd [concat git hook run --ignore-missing $hook_name -- $args 2>@1] - return [_open_stdout_stderr $cmd] + git_read hook run --ignore-missing $hook_name -- $args 2>@1 } proc kill_file_process {fd} { From 67a128b91e25978a15f9f7e194d81b441d603652 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Sat, 29 Mar 2025 18:49:05 +0100 Subject: [PATCH 34/51] gitk: sanitize 'open' arguments: revisit recently updated 'open' calls The previous commits bb5cb23daf75 (gitk: prevent overly long command lines, 2023-01-24) rewrote a set of the 'open' calls substantially. These were then later updated by 7dd272eca153 (gitk: escape file paths before piping to git log, 2023-01-24) and d5d1b91e5327 (gitk: encode arguments correctly with "open", 2025-03-07). In the preceding merge, the conversions to a safe_open variant were undone to ensure that the principal operation of the new 'open' calls is not modified by accident. Since the 'open' calls now pass a redirection from a Tcl string as stdin, convert the calls to 'safe_open_command_redirect'. Signed-off-by: Johannes Sixt Signed-off-by: Taylor Blau --- gitk | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/gitk b/gitk index 2ab2ccb2b3..c3bf6da882 100755 --- a/gitk +++ b/gitk @@ -498,9 +498,9 @@ proc start_rev_list {view} { } if {[catch { - set fd [open [concat | git log --no-color -z --pretty=raw $show_notes \ - --parents --boundary $args --stdin \ - [list "<<[join [concat $revs "--" $files] "\n"]"]] r] + set fd [safe_open_command_redirect [concat git log --no-color -z --pretty=raw $show_notes \ + --parents --boundary $args --stdin] \ + [list "<<[join [concat $revs "--" $files] "\n"]"]] } err]} { error_popup "[mc "Error executing git log:"] $err" return 0 @@ -651,9 +651,9 @@ proc updatecommits {} { set args $vorigargs($view) } if {[catch { - set fd [open [concat | git log --no-color -z --pretty=raw $show_notes \ - --parents --boundary $args --stdin \ - [list "<<[join [concat $revs "--" $vfilelimit($view)] "\n"]"]] r] + set fd [safe_open_command_redirect [concat git log --no-color -z --pretty=raw $show_notes \ + --parents --boundary $args --stdin] \ + [list "<<[join [concat $revs "--" $vfilelimit($view)] "\n"]"]] } err]} { error_popup "[mc "Error executing git log:"] $err" return @@ -10322,10 +10322,11 @@ proc getallcommits {} { if {$ids ne {}} { if {$ids eq "--all"} { set cmd [concat $cmd "--all"] + set fd [safe_open_command $cmd] } else { - set cmd [concat $cmd --stdin [list "<<[join $ids "\n"]"]] + set cmd [concat $cmd --stdin] + set fd [safe_open_command_redirect $cmd [list "<<[join $ids "\n"]"]] } - set fd [open $cmd r] fconfigure $fd -blocking 0 incr allcommits nowbusy allcommits From a1ccd2512072cf52835050f4c97a4fba9f0ec8f9 Mon Sep 17 00:00:00 2001 From: Mark Levedahl Date: Fri, 11 Apr 2025 10:58:20 -0400 Subject: [PATCH 35/51] 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.sh | 122 +++++++++++++++++++++++++++++------------------------ 1 file changed, 66 insertions(+), 56 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index 217aeb9ce3..21c3858d2e 100755 --- a/git-gui.sh +++ b/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]} { + 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 - } - - foreach p $_search_path { - set p [file join $p $what$suffix] - if {[file exists $p]} { - return [file normalize $p] + 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 } - } - 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 {[llength [file split $cmd]] < 2} { - set fullpath [_which $cmd] - if {$fullpath eq ""} { - throw {NOT-FOUND} "$cmd not found in PATH" + foreach p $_search_path { + set p [file join $p $what$suffix] + if {[file exists $p]} { + return [file normalize $p] } - lset command_line $i $fullpath } + return {} + } - # handle piped commands, e.g. `exec A | B` - for {incr i} {$i < [llength $command_line]} {incr i} { - if {[lindex $command_line $i] eq "|"} { + 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 + } + } + } + return $command_line + } + + # Override `exec` to avoid unsafe PATH lookup + + rename exec real_exec + + 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 +} 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. -rename open real_open - -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 } ###################################################################### From dc9ecb1aab1a3438fceeb44db67ddf8e8d938324 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Sat, 3 May 2025 13:24:48 +0200 Subject: [PATCH 36/51] 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.sh | 48 ++++++++++++++++++------------------ lib/blame.tcl | 10 ++++---- lib/branch.tcl | 6 ++--- lib/browser.tcl | 2 +- lib/checkout_op.tcl | 10 ++++---- lib/choose_repository.tcl | 8 +++--- lib/choose_rev.tcl | 6 ++--- lib/commit.tcl | 6 ++--- lib/console.tcl | 2 +- lib/database.tcl | 2 +- lib/diff.tcl | 8 +++--- lib/index.tcl | 8 +++--- lib/merge.tcl | 2 +- lib/mergetool.tcl | 2 +- lib/remote.tcl | 2 +- lib/remote_branch_delete.tcl | 2 +- 16 files changed, 62 insertions(+), 62 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index e10bf2a039..301c7647ec 100755 --- a/git-gui.sh +++ b/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,34 +642,34 @@ 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} { - git_read hook run --ignore-missing $hook_name -- $args 2>@1 + git_read [concat [list hook run --ignore-missing $hook_name --] $args 2>@1] } 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/lib/blame.tcl b/lib/blame.tcl index ceb2330266..d6fd8bea91 100644 --- a/lib/blame.tcl +++ b/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/lib/branch.tcl b/lib/branch.tcl index 8b0c485889..39e0f2dc98 100644 --- a/lib/branch.tcl +++ b/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/lib/browser.tcl b/lib/browser.tcl index a982983667..6fc8d4d637 100644 --- a/lib/browser.tcl +++ b/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/lib/checkout_op.tcl b/lib/checkout_op.tcl index 31992f461b..48fd1a3cac 100644 --- a/lib/checkout_op.tcl +++ b/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/lib/choose_repository.tcl b/lib/choose_repository.tcl index 7bd738e51e..7b64a11239 100644 --- a/lib/choose_repository.tcl +++ b/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/lib/choose_rev.tcl b/lib/choose_rev.tcl index 7cf9e160fa..8ae7e8a5c4 100644 --- a/lib/choose_rev.tcl +++ b/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/lib/commit.tcl b/lib/commit.tcl index 8d135845a5..b27e37d385 100644 --- a/lib/commit.tcl +++ b/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/lib/console.tcl b/lib/console.tcl index c7f20b87cb..44dcdf29be 100644 --- a/lib/console.tcl +++ b/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/lib/database.tcl b/lib/database.tcl index 85783081e0..1fc0ea00b3 100644 --- a/lib/database.tcl +++ b/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/lib/diff.tcl b/lib/diff.tcl index ce1bad6900..8ec740eb50 100644 --- a/lib/diff.tcl +++ b/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/lib/index.tcl b/lib/index.tcl index d2ec24bd80..857864ff2b 100644 --- a/lib/index.tcl +++ b/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/lib/merge.tcl b/lib/merge.tcl index e97c91eab6..6317c32127 100644 --- a/lib/merge.tcl +++ b/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/lib/mergetool.tcl b/lib/mergetool.tcl index f2fa439305..777d7b323b 100644 --- a/lib/mergetool.tcl +++ b/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/lib/remote.tcl b/lib/remote.tcl index a733a0f36e..cf796d1601 100644 --- a/lib/remote.tcl +++ b/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/lib/remote_branch_delete.tcl b/lib/remote_branch_delete.tcl index 5ba9fcadd1..c8c99b17a8 100644 --- a/lib/remote_branch_delete.tcl +++ b/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/51] 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.sh | 10 +++++----- lib/console.tcl | 4 ++-- lib/mergetool.tcl | 4 ++-- lib/sshkey.tcl | 2 +- lib/tools.tcl | 3 +-- 5 files changed, 11 insertions(+), 12 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index 301c7647ec..408149b530 100755 --- a/git-gui.sh +++ b/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/lib/console.tcl b/lib/console.tcl index 44dcdf29be..cc416d4811 100644 --- a/lib/console.tcl +++ b/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/lib/mergetool.tcl b/lib/mergetool.tcl index 777d7b323b..6b26726418 100644 --- a/lib/mergetool.tcl +++ b/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/lib/sshkey.tcl b/lib/sshkey.tcl index 2e006cb8ca..b32bdd06e9 100644 --- a/lib/sshkey.tcl +++ b/lib/sshkey.tcl @@ -85,7 +85,7 @@ proc make_ssh_key {w} { set cmdline [list sh -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/lib/tools.tcl b/lib/tools.tcl index 413f1a1700..142ffceedd 100644 --- a/lib/tools.tcl +++ b/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/51] 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.sh | 6 +++--- lib/checkout_op.tcl | 4 ++-- lib/choose_repository.tcl | 4 ++-- lib/console.tcl | 3 +-- lib/merge.tcl | 2 +- 5 files changed, 9 insertions(+), 10 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index 408149b530..bbdbd35d26 100755 --- a/git-gui.sh +++ b/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} { @@ -669,7 +669,7 @@ proc git_write {cmd} { } proc githook_read {hook_name args} { - git_read [concat [list hook run --ignore-missing $hook_name --] $args 2>@1] + git_read [concat [list hook run --ignore-missing $hook_name --] $args] [list 2>@1] } proc kill_file_process {fd} { diff --git a/lib/checkout_op.tcl b/lib/checkout_op.tcl index 48fd1a3cac..87ed0b4858 100644 --- a/lib/checkout_op.tcl +++ b/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/lib/choose_repository.tcl b/lib/choose_repository.tcl index 7b64a11239..5b361cc424 100644 --- a/lib/choose_repository.tcl +++ b/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/lib/console.tcl b/lib/console.tcl index cc416d4811..4715ce91e6 100644 --- a/lib/console.tcl +++ b/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/lib/merge.tcl b/lib/merge.tcl index 6317c32127..55b4fc5ed3 100644 --- a/lib/merge.tcl +++ b/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/51] 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.sh | 8 ++++++-- lib/commit.tcl | 4 ++-- lib/merge.tcl | 2 +- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index bbdbd35d26..75d7b9b9fc 100755 --- a/git-gui.sh +++ b/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/lib/commit.tcl b/lib/commit.tcl index b27e37d385..bb6056d0ad 100644 --- a/lib/commit.tcl +++ b/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/lib/merge.tcl b/lib/merge.tcl index 55b4fc5ed3..44c3f93584 100644 --- a/lib/merge.tcl +++ b/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/51] 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.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.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/git-gui.sh b/git-gui.sh index 75d7b9b9fc..9ad172ac66 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -600,6 +600,7 @@ proc open_cmd_pipe {cmd path} { } else { 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/51] 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.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index 0355c0c836..2c446c2784 100755 --- a/git-gui.sh +++ b/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 { From 05e9cd64ee23bbadcea6bcffd6660ed02b8eab89 Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Mon, 19 May 2025 21:26:04 -0500 Subject: [PATCH 42/51] config: quote values containing CR character When reading the config, values that contain a trailing CRLF are stripped. If the value itself has a trailing CR, the normal LF that follows results in the CR being unintentionally stripped. This may lead to unintended behavior due to the config value written being different when it gets read. One such issue involves a repository with a submodule path containing a trailing CR. When the submodule gets initialized, the submodule is cloned without being checked out and has "core.worktree" set to the submodule path. The git-checkout(1) that gets spawned later reads the "core.worktree" config value, but without the trailing CR, and consequently attempts to checkout to a different path than intended. If the repository contains a matching path that is a symlink, it is possible for the submodule repository to be checked out in arbitrary locations. This is extra bad when the symlink points to the submodule hooks directory and the submodule repository contains an executable "post-checkout" hook. Once the submodule repository checkout completes, the "post-checkout" hook immediately executes. To prevent mismatched config state due to misinterpreting a trailing CR, wrap config values containing CR in double quotes when writing the entry. This ensures a trailing CR is always separated for an LF and thus prevented from getting stripped. Note that this problem cannot be addressed by just quoting each CR with "\r". The reading side of the config interprets only a few backslash escapes, and "\r" is not among them. This fix is sufficient though because it only affects the CR at the end of a line and any literal CR in the interior is already preserved. Co-authored-by: David Leadbeater Signed-off-by: Justin Tobler Signed-off-by: Taylor Blau --- config.c | 2 +- t/t1300-config.sh | 11 +++++++++++ t/t7450-bad-git-dotfiles.sh | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 1 deletion(-) diff --git a/config.c b/config.c index 9ff6ae1cb9..629981451d 100644 --- a/config.c +++ b/config.c @@ -2999,7 +2999,7 @@ static ssize_t write_pair(int fd, const char *key, const char *value, if (value[0] == ' ') quote = "\""; for (i = 0; value[i]; i++) - if (value[i] == ';' || value[i] == '#') + if (value[i] == ';' || value[i] == '#' || value[i] == '\r') quote = "\""; if (i && value[i - 1] == ' ') quote = "\""; diff --git a/t/t1300-config.sh b/t/t1300-config.sh index f4e2752134..1010410b7e 100755 --- a/t/t1300-config.sh +++ b/t/t1300-config.sh @@ -2590,4 +2590,15 @@ test_expect_success 'includeIf.hasconfig:remote.*.url forbids remote url in such grep "fatal: remote URLs cannot be configured in file directly or indirectly included by includeIf.hasconfig:remote.*.url" err ' +test_expect_success 'writing value with trailing CR not stripped on read' ' + test_when_finished "rm -rf cr-test" && + + printf "bar\r\n" >expect && + git init cr-test && + git -C cr-test config set core.foo $(printf "bar\r") && + git -C cr-test config get core.foo >actual && + + test_cmp expect actual +' + test_done diff --git a/t/t7450-bad-git-dotfiles.sh b/t/t7450-bad-git-dotfiles.sh index 5b845e899b..2026285566 100755 --- a/t/t7450-bad-git-dotfiles.sh +++ b/t/t7450-bad-git-dotfiles.sh @@ -347,4 +347,37 @@ test_expect_success 'checkout -f --recurse-submodules must not use a nested gitd test_path_is_missing nested_checkout/thing2/.git ' +test_expect_success SYMLINKS,!WINDOWS,!MINGW 'submodule must not checkout into different directory' ' + test_when_finished "rm -rf sub repo bad-clone" && + + git init sub && + write_script sub/post-checkout <<-\EOF && + touch "$PWD/foo" + EOF + git -C sub add post-checkout && + git -C sub commit -m hook && + + git init repo && + git -C repo -c protocol.file.allow=always submodule add "$PWD/sub" sub && + git -C repo mv sub $(printf "sub\r") && + + # Ensure config values containing CR are wrapped in quotes. + git config unset -f repo/.gitmodules submodule.sub.path && + printf "\tpath = \"sub\r\"\n" >>repo/.gitmodules && + + git config unset -f repo/.git/modules/sub/config core.worktree && + { + printf "[core]\n" && + printf "\tworktree = \"../../../sub\r\"\n" + } >>repo/.git/modules/sub/config && + + ln -s .git/modules/sub/hooks repo/sub && + git -C repo add -A && + git -C repo commit -m submodule && + + git -c protocol.file.allow=always clone --recurse-submodules repo bad-clone && + ! test -f "$PWD/foo" && + test -f $(printf "bad-clone/sub\r/post-checkout") +' + test_done From 35cb1bb0b92c132249d932c05bbd860d410e12d4 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt' via Git Security Date: Wed, 14 May 2025 08:32:02 +0200 Subject: [PATCH 43/51] bundle-uri: fix arbitrary file writes via parameter injection We fetch bundle URIs via `download_https_uri_to_file()`. The logic to fetch those bundles is not handled in-process, but we instead use a separate git-remote-https(1) process that performs the fetch for us. The information about which file should be downloaded and where that file should be put gets communicated via stdin of that process via a "get" request. This "get" request has the form "get $uri $file\n\n". As may be obvious to the reader, this will cause git-remote-https(1) to download the URI "$uri" and put it into "$file". The fact that we are using plain spaces and newlines as separators for the request arguments means that we have to be extra careful with the respective vaules of these arguments: - If "$uri" contained a space we would interpret this as both URI and target location. - If either "$uri" or "$file" contained a newline we would interpret this as a new command. But we neither quote the arguments such that any characters with special meaning would be escaped, nor do we verify that none of these special characters are contained. If either the URI or file contains a newline character, we are open to protocol injection attacks. Likewise, if the URI itself contains a space, then an attacker-controlled URI can lead to partially-controlled file writes. Note that the attacker-controlled URIs do not permit completely arbitrary file writes, but instead allows an attacker to control the path in which we will write a temporary (e.g., "tmp_uri_XXXXXX") file. The result is twofold: - By adding a space in "$uri" we can control where exactly a file will be written to, including out-of-repository writes. The final location is not completely arbitrary, as the injected string will be concatenated with the original "$file" path. Furthermore, the name of the bundle will be "tmp_uri_XXXXXX", further restricting what an adversary would be able to write. Also note that is not possible for the URI to contain a newline because we end up in `credential_from_url_1()` before we try to issue any requests using that URI. As such, it is not possible to inject arbitrary commands via the URI. - By adding a newline to "$file" we can inject arbitrary commands. This gives us full control over where a specific file will be written to. Potential attack vectors would be to overwrite hooks, but if an adversary were to guess where the user's home directory is located they might also easily write e.g. a "~/.profile" file and thus cause arbitrary code execution. This injection can only become possible when the adversary has full control over the target path where a bundle will be downloaded to. While this feels unlikely, it is possible to control this path when users perform a recursive clone with a ".gitmodules" file that is controlled by the adversary. Luckily though, the use of bundle URIs is not enabled by default in Git clients (yet): they have to be enabled by setting the `bundle.heuristic` config key explicitly. As such, the blast radius of this parameter injection should overall be quite contained. Fix the issue by rejecting spaces in the URI and newlines in both the URI and the file. As explained, it shouldn't be required to also restrict the use of newlines in the URI, as we would eventually die anyway in `credential_from_url_1()`. But given that we're only one small step away from arbitrary code execution, let's rather be safe and restrict newlines in URIs, as well. Eventually we should probably refactor the way that Git talks with the git-remote-https(1) subprocess so that it is less fragile. Until then, these two restrictions should plug the issue. Reported-by: David Leadbeater Based-on-patch-by: David Leadbeater Signed-off-by: Patrick Steinhardt Signed-off-by: Taylor Blau --- bundle-uri.c | 22 ++++++++++++++++++++++ t/t5558-clone-bundle-uri.sh | 23 +++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/bundle-uri.c b/bundle-uri.c index ca32050a78..a6a3c1c4b3 100644 --- a/bundle-uri.c +++ b/bundle-uri.c @@ -292,6 +292,28 @@ static int download_https_uri_to_file(const char *file, const char *uri) struct strbuf line = STRBUF_INIT; int found_get = 0; + /* + * The protocol we speak with git-remote-https(1) uses a space to + * separate between URI and file, so the URI itself must not contain a + * space. If it did, an adversary could change the location where the + * downloaded file is being written to. + * + * Similarly, we use newlines to separate commands from one another. + * Consequently, neither the URI nor the file must contain a newline or + * otherwise an adversary could inject arbitrary commands. + * + * TODO: Restricting newlines in the target paths may break valid + * usecases, even if those are a bit more on the esoteric side. + * If this ever becomes a problem we should probably think about + * alternatives. One alternative could be to use NUL-delimited + * requests in git-remote-http(1). Another alternative could be + * to use URL quoting. + */ + if (strpbrk(uri, " \n")) + return error("bundle-uri: URI is malformed: '%s'", file); + if (strchr(file, '\n')) + return error("bundle-uri: filename is malformed: '%s'", file); + strvec_pushl(&cp.args, "git-remote-https", uri, NULL); cp.err = -1; cp.in = -1; diff --git a/t/t5558-clone-bundle-uri.sh b/t/t5558-clone-bundle-uri.sh index 996a08e90c..2af523aaa4 100755 --- a/t/t5558-clone-bundle-uri.sh +++ b/t/t5558-clone-bundle-uri.sh @@ -1052,6 +1052,29 @@ test_expect_success 'bundles are downloaded once during fetch --all' ' trace-mult.txt >bundle-fetches && test_line_count = 1 bundle-fetches ' + +test_expect_success 'bundles with space in URI are rejected' ' + test_when_finished "rm -rf busted repo" && + mkdir -p "$HOME/busted/ /$HOME/repo/.git/objects/bundles" && + git clone --bundle-uri="$HTTPD_URL/bogus $HOME/busted/" "$HTTPD_URL/smart/fetch.git" repo 2>err && + test_grep "error: bundle-uri: URI is malformed: " err && + find busted -type f >files && + test_must_be_empty files +' + +test_expect_success 'bundles with newline in URI are rejected' ' + test_when_finished "rm -rf busted repo" && + git clone --bundle-uri="$HTTPD_URL/bogus\nget $HTTPD_URL/bogus $HOME/busted" "$HTTPD_URL/smart/fetch.git" repo 2>err && + test_grep "error: bundle-uri: URI is malformed: " err && + test_path_is_missing "$HOME/busted" +' + +test_expect_success 'bundles with newline in target path are rejected' ' + git clone --bundle-uri="$HTTPD_URL/bogus" "$HTTPD_URL/smart/fetch.git" "$(printf "escape\nget $HTTPD_URL/bogus .")" 2>err && + test_grep "error: bundle-uri: filename is malformed: " err && + test_path_is_missing escape +' + # Do not add tests here unless they use the HTTP server, as they will # not run unless the HTTP dependencies exist. From 9de345cb273cc7faaeda279c7e07149d8a15a319 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Mon, 19 May 2025 18:30:29 -0400 Subject: [PATCH 44/51] wincred: avoid buffer overflow in wcsncat() The wincred credential helper uses a static buffer ("target") as a unique key for storing and comparing against internal storage. It does this by building up a string is supposed to look like: git:$PROTOCOL://$USERNAME@$HOST/@PATH However, the static "target" buffer is declared as a wide string with no more than 1,024 wide characters. The first call to wcsncat() is almost correct (it copies no more than ARRAY_SIZE(target) wchar_t's), but does not account for the trailing NUL, introducing an off-by-one error. But subsequent calls to wcsncat() have an additional problem on top of the off-by-one. They do not account for the length of the existing wide string being built up in 'target'. So the following: $ perl -e ' my $x = "x" x 1_000; print "protocol=$x\nhost=$x\nusername=$x\npath=$x\n" ' | C\:/Program\ Files/Git/mingw64/libexec/git-core/git-credential-wincred.exe get will result in a segmentation fault from over-filling buffer. This bug is as old as the wincred helper itself, dating back to a6253da0f3 (contrib: add win32 credential-helper, 2012-07-27). Commit 8b2d219a3d (wincred: improve compatibility with windows versions, 2013-01-10) replaced the use of strncat() with wcsncat(), but retained the buggy behavior. Fix this by using a "target_append()" helper which accounts for both the length of the existing string within the buffer, as well as the trailing NUL character. Reported-by: David Leadbeater Helped-by: David Leadbeater Helped-by: Jeff King Signed-off-by: Taylor Blau --- .../wincred/git-credential-wincred.c | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c index 4cd56c42e2..ceff44207a 100644 --- a/contrib/credential/wincred/git-credential-wincred.c +++ b/contrib/credential/wincred/git-credential-wincred.c @@ -37,6 +37,14 @@ static void *xmalloc(size_t size) static WCHAR *wusername, *password, *protocol, *host, *path, target[1024], *password_expiry_utc; +static void target_append(const WCHAR *src) +{ + size_t avail = ARRAY_SIZE(target) - wcslen(target) - 1; /* -1 for NUL */ + if (avail < wcslen(src)) + die("target buffer overflow"); + wcsncat(target, src, avail); +} + static void write_item(const char *what, LPCWSTR wbuf, int wlen) { char *buf; @@ -294,17 +302,17 @@ int main(int argc, char *argv[]) /* prepare 'target', the unique key for the credential */ wcscpy(target, L"git:"); - wcsncat(target, protocol, ARRAY_SIZE(target)); - wcsncat(target, L"://", ARRAY_SIZE(target)); + target_append(protocol); + target_append(L"://"); if (wusername) { - wcsncat(target, wusername, ARRAY_SIZE(target)); - wcsncat(target, L"@", ARRAY_SIZE(target)); + target_append(wusername); + target_append(L"@"); } if (host) - wcsncat(target, host, ARRAY_SIZE(target)); + target_append(host); if (path) { - wcsncat(target, L"/", ARRAY_SIZE(target)); - wcsncat(target, path, ARRAY_SIZE(target)); + target_append(L"/"); + target_append(path); } if (!strcmp(argv[1], "get")) From 7a1903ad46b5cc7524c0734a5034dccaec07209b Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Wed, 28 May 2025 14:42:12 -0400 Subject: [PATCH 45/51] Git 2.43.7 Signed-off-by: Taylor Blau --- Documentation/RelNotes/2.43.7.txt | 73 +++++++++++++++++++++++++++++++ GIT-VERSION-GEN | 2 +- RelNotes | 2 +- 3 files changed, 75 insertions(+), 2 deletions(-) create mode 100644 Documentation/RelNotes/2.43.7.txt diff --git a/Documentation/RelNotes/2.43.7.txt b/Documentation/RelNotes/2.43.7.txt new file mode 100644 index 0000000000..95702a036e --- /dev/null +++ b/Documentation/RelNotes/2.43.7.txt @@ -0,0 +1,73 @@ +Git v2.43.7 Release Notes +========================= + +This release includes fixes for CVE-2025-27613, CVE-2025-27614, +CVE-2025-46334, CVE-2025-46835, CVE-2025-48384, CVE-2025-48385, and +CVE-2025-48386. + +Fixes since v2.43.6 +------------------- + + * CVE-2025-27613, Gitk: + + When a user clones an untrusted repository and runs Gitk without + additional command arguments, any writable file can be created and + truncated. The option "Support per-file encoding" must have been + enabled. The operation "Show origin of this line" is affected as + well, regardless of the option being enabled or not. + + * CVE-2025-27614, Gitk: + + A Git repository can be crafted in such a way that a user who has + cloned the repository can be tricked into running any script + supplied by the attacker by invoking `gitk filename`, where + `filename` has a particular structure. + + * CVE-2025-46334, Git GUI (Windows only): + + A malicious repository can ship versions of sh.exe or typical + textconv filter programs such as astextplain. On Windows, path + lookup can find such executables in the worktree. These programs + are invoked when the user selects "Git Bash" or "Browse Files" from + the menu. + + * CVE-2025-46835, Git GUI: + + When a user clones an untrusted repository and is tricked into + editing a file located in a maliciously named directory in the + repository, then Git GUI can create and overwrite any writable + file. + + * CVE-2025-48384, Git: + + When reading a config value, Git strips any trailing carriage + return and line feed (CRLF). When writing a config entry, values + with a trailing CR are not quoted, causing the CR to be lost when + the config is later read. When initializing a submodule, if the + submodule path contains a trailing CR, the altered path is read + resulting in the submodule being checked out to an incorrect + location. If a symlink exists that points the altered path to the + submodule hooks directory, and the submodule contains an executable + post-checkout hook, the script may be unintentionally executed + after checkout. + + * CVE-2025-48385, Git: + + When cloning a repository Git knows to optionally fetch a bundle + advertised by the remote server, which allows the server-side to + offload parts of the clone to a CDN. The Git client does not + perform sufficient validation of the advertised bundles, which + allows the remote side to perform protocol injection. + + This protocol injection can cause the client to write the fetched + bundle to a location controlled by the adversary. The fetched + content is fully controlled by the server, which can in the worst + case lead to arbitrary code execution. + + * CVE-2025-48386, Git: + + The wincred credential helper uses a static buffer (`target`) as a + unique key for storing and comparing against internal storage. This + credential helper does not properly bounds check the available + space remaining in the buffer before appending to it with + `wcsncat()`, leading to potential buffer overflows. diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index 81630dde84..2c60bf7588 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,7 +1,7 @@ #!/bin/sh GVF=GIT-VERSION-FILE -DEF_VER=v2.43.6 +DEF_VER=v2.43.7 LF=' ' diff --git a/RelNotes b/RelNotes index 0a9d8a03d7..fb4fb87bbf 120000 --- a/RelNotes +++ b/RelNotes @@ -1 +1 @@ -Documentation/RelNotes/2.43.6.txt \ No newline at end of file +Documentation/RelNotes/2.43.7.txt \ No newline at end of file From 080b728d4b2bbdd2c0b9eb9ed6a41195f8303088 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Wed, 28 May 2025 14:51:12 -0400 Subject: [PATCH 46/51] Git 2.44.4 Signed-off-by: Taylor Blau --- Documentation/RelNotes/2.44.4.txt | 7 +++++++ GIT-VERSION-GEN | 2 +- RelNotes | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 Documentation/RelNotes/2.44.4.txt diff --git a/Documentation/RelNotes/2.44.4.txt b/Documentation/RelNotes/2.44.4.txt new file mode 100644 index 0000000000..8db4d5b537 --- /dev/null +++ b/Documentation/RelNotes/2.44.4.txt @@ -0,0 +1,7 @@ +Git v2.44.4 Release Notes +========================= + +This release merges up the fixes that appears in v2.43.7 to address +the following CVEs: CVE-2025-27613, CVE-2025-27614, CVE-2025-46334, +CVE-2025-46835, CVE-2025-48384, CVE-2025-48385, and CVE-2025-48386. +See the release notes for v2.43.7 for details. diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index 33476e262d..8a9fdd06d7 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,7 +1,7 @@ #!/bin/sh GVF=GIT-VERSION-FILE -DEF_VER=v2.44.3 +DEF_VER=v2.44.4 LF=' ' diff --git a/RelNotes b/RelNotes index 509eba5f1a..4437ce1347 120000 --- a/RelNotes +++ b/RelNotes @@ -1 +1 @@ -Documentation/RelNotes/2.44.3.txt \ No newline at end of file +Documentation/RelNotes/2.44.4.txt \ No newline at end of file From f94b90ad6e49cc7f15c4171c5a434aa459e82d2d Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Wed, 28 May 2025 14:54:04 -0400 Subject: [PATCH 47/51] Git 2.45.4 Signed-off-by: Taylor Blau --- Documentation/RelNotes/2.45.4.txt | 7 +++++++ GIT-VERSION-GEN | 2 +- RelNotes | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 Documentation/RelNotes/2.45.4.txt diff --git a/Documentation/RelNotes/2.45.4.txt b/Documentation/RelNotes/2.45.4.txt new file mode 100644 index 0000000000..5b50d8daf0 --- /dev/null +++ b/Documentation/RelNotes/2.45.4.txt @@ -0,0 +1,7 @@ +Git v2.45.4 Release Notes +========================= + +This release merges up the fixes that appears in v2.43.7, and v2.44.4 +to address the following CVEs: CVE-2025-27613, CVE-2025-27614, +CVE-2025-46334, CVE-2025-46835, CVE-2025-48384, CVE-2025-48385, and +CVE-2025-48386. See the release notes for v2.43.7 for details. diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index f7c5d8f070..277d3715fb 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,7 +1,7 @@ #!/bin/sh GVF=GIT-VERSION-FILE -DEF_VER=v2.45.3 +DEF_VER=v2.45.4 LF=' ' diff --git a/RelNotes b/RelNotes index 36d6ed4435..d915c8e057 120000 --- a/RelNotes +++ b/RelNotes @@ -1 +1 @@ -Documentation/RelNotes/2.45.3.txt \ No newline at end of file +Documentation/RelNotes/2.45.4.txt \ No newline at end of file From 47d3b506d48b7971080f09770f5b06b42569c967 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Wed, 28 May 2025 14:58:48 -0400 Subject: [PATCH 48/51] Git 2.46.4 Signed-off-by: Taylor Blau --- Documentation/RelNotes/2.46.4.txt | 7 +++++++ GIT-VERSION-GEN | 2 +- RelNotes | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 Documentation/RelNotes/2.46.4.txt diff --git a/Documentation/RelNotes/2.46.4.txt b/Documentation/RelNotes/2.46.4.txt new file mode 100644 index 0000000000..622f4c752f --- /dev/null +++ b/Documentation/RelNotes/2.46.4.txt @@ -0,0 +1,7 @@ +Git v2.46.4 Release Notes +========================= + +This release merges up the fixes that appears in v2.43.7, v2.44.4, and +v2.45.4 to address the following CVEs: CVE-2025-27613, CVE-2025-27614, +CVE-2025-46334, CVE-2025-46835, CVE-2025-48384, CVE-2025-48385, and +CVE-2025-48386. See the release notes for v2.43.7 for details. diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index 7cde816ede..0c7a842fc5 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,7 +1,7 @@ #!/bin/sh GVF=GIT-VERSION-FILE -DEF_VER=v2.46.3 +DEF_VER=v2.46.4 LF=' ' diff --git a/RelNotes b/RelNotes index 686ec8709b..1cdc9ff7d0 120000 --- a/RelNotes +++ b/RelNotes @@ -1 +1 @@ -Documentation/RelNotes/2.46.3.txt \ No newline at end of file +Documentation/RelNotes/2.46.4.txt \ No newline at end of file From a52a24e03c8c711f1d5e252fba78f9276908129b Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Wed, 28 May 2025 15:16:03 -0400 Subject: [PATCH 49/51] Git 2.47.3 Signed-off-by: Taylor Blau --- Documentation/RelNotes/2.47.3.txt | 8 ++++++++ GIT-VERSION-GEN | 2 +- RelNotes | 2 +- 3 files changed, 10 insertions(+), 2 deletions(-) create mode 100644 Documentation/RelNotes/2.47.3.txt diff --git a/Documentation/RelNotes/2.47.3.txt b/Documentation/RelNotes/2.47.3.txt new file mode 100644 index 0000000000..bc2a2b833b --- /dev/null +++ b/Documentation/RelNotes/2.47.3.txt @@ -0,0 +1,8 @@ +Git v2.47.3 Release Notes +========================= + +This release merges up the fixes that appears in v2.43.7, v2.44.4, +v2.45.4, and v2.46.4 to address the following CVEs: CVE-2025-27613, +CVE-2025-27614, CVE-2025-46334, CVE-2025-46835, CVE-2025-48384, +CVE-2025-48385, and CVE-2025-48386. See the release notes for v2.43.7 +for details. diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index 5fcb9ded7f..5382ff4f89 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,7 +1,7 @@ #!/bin/sh GVF=GIT-VERSION-FILE -DEF_VER=v2.47.1 +DEF_VER=v2.47.3 LF=' ' diff --git a/RelNotes b/RelNotes index 768c16d81b..cc01df8574 120000 --- a/RelNotes +++ b/RelNotes @@ -1 +1 @@ -Documentation/RelNotes/2.47.1.txt \ No newline at end of file +Documentation/RelNotes/2.47.3.txt \ No newline at end of file From fbae1f06cbb04a6592c32f465e9bc28149039358 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Wed, 28 May 2025 15:18:19 -0400 Subject: [PATCH 50/51] Git 2.48.2 Signed-off-by: Taylor Blau --- Documentation/RelNotes/2.48.2.txt | 8 ++++++++ GIT-VERSION-GEN | 2 +- RelNotes | 2 +- 3 files changed, 10 insertions(+), 2 deletions(-) create mode 100644 Documentation/RelNotes/2.48.2.txt diff --git a/Documentation/RelNotes/2.48.2.txt b/Documentation/RelNotes/2.48.2.txt new file mode 100644 index 0000000000..f3f2f90c2b --- /dev/null +++ b/Documentation/RelNotes/2.48.2.txt @@ -0,0 +1,8 @@ +Git v2.48.2 Release Notes +========================= + +This release merges up the fixes that appears in v2.43.7, v2.44.4, +v2.45.4, v2.46.4, and v2.47.3 to address the following CVEs: +CVE-2025-27613, CVE-2025-27614, CVE-2025-46334, CVE-2025-46835, +CVE-2025-48384, CVE-2025-48385, and CVE-2025-48386. See the release +notes for v2.43.7 for details. diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index 570cc11622..df11f29d8d 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,6 +1,6 @@ #!/bin/sh -DEF_VER=v2.48.1 +DEF_VER=v2.48.2 LF=' ' diff --git a/RelNotes b/RelNotes index f28189867b..87db0ec215 120000 --- a/RelNotes +++ b/RelNotes @@ -1 +1 @@ -Documentation/RelNotes/2.48.1.txt \ No newline at end of file +Documentation/RelNotes/2.48.2.txt \ No newline at end of file From aadf8ae518afd80b73d49eff8aff475161aa5157 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 13 Jun 2025 07:51:58 -0700 Subject: [PATCH 51/51] Git 2.49.1 Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.49.1.txt | 12 ++++++++++++ GIT-VERSION-GEN | 2 +- RelNotes | 2 +- 3 files changed, 14 insertions(+), 2 deletions(-) create mode 100644 Documentation/RelNotes/2.49.1.txt diff --git a/Documentation/RelNotes/2.49.1.txt b/Documentation/RelNotes/2.49.1.txt new file mode 100644 index 0000000000..c619e8b495 --- /dev/null +++ b/Documentation/RelNotes/2.49.1.txt @@ -0,0 +1,12 @@ +Git v2.49.1 Release Notes +========================= + +This release merges up the fixes that appear in v2.43.7, v2.44.4, +v2.45.4, v2.46.4, v2.47.3, and v2.48.2 to address the following CVEs: +CVE-2025-27613, CVE-2025-27614, CVE-2025-46334, CVE-2025-46835, +CVE-2025-48384, CVE-2025-48385, and CVE-2025-48386. See the release +notes for v2.43.7 for details. + +It also contains some updates to various CI bits to work around +and/or to adjust to the deprecation of use of Ubuntu 20.04 GitHub +Actions CI, updates to to Fedora base image. diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index 3abfe7d3d7..4b2c88140e 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,6 +1,6 @@ #!/bin/sh -DEF_VER=v2.49.0 +DEF_VER=v2.49.1 LF=' ' diff --git a/RelNotes b/RelNotes index ac72bdf04d..e8a7222a34 120000 --- a/RelNotes +++ b/RelNotes @@ -1 +1 @@ -Documentation/RelNotes/2.49.0.adoc \ No newline at end of file +Documentation/RelNotes/2.49.1.txt \ No newline at end of file