Skip to content

Commit 3f07230

Browse files
committed
Merge branch 'js/fix-open-exec-git'
This addresses CVE-2025-46835, Git GUI can create and overwrite a user's files: 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 files for which the user has write permission. * js/fix-open-exec-git: git-gui: sanitize 'exec' arguments: convert new 'cygpath' calls git-gui: do not mistake command arguments as redirection operators git-gui: introduce function git_redir for git calls with redirections git-gui: pass redirections as separate argument to git_read git-gui: pass redirections as separate argument to _open_stdout_stderr git-gui: convert git_read*, git_write to be non-variadic git-gui: use git_read in githook_read git-gui: break out a separate function git_read_nice git-gui: remove option --stderr from git_read git-gui: sanitize 'exec' arguments: background git-gui: sanitize 'exec' arguments: simple cases git-gui: treat file names beginning with "|" as relative paths git-gui: remove git config --list handling for git < 1.5.3 git-gui: remove HEAD detachment implementation for git < 1.5.3 git-gui: remove Tcl 8.4 workaround on 2>@1 redirection Signed-off-by: Johannes Sixt <[email protected]>
2 parents 88125ff + a437f5b commit 3f07230

20 files changed

+218
-234
lines changed

git-gui.sh

Lines changed: 138 additions & 145 deletions
Large diffs are not rendered by default.

lib/blame.tcl

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -481,14 +481,14 @@ method _load {jump} {
481481
if {$do_textconv ne 0} {
482482
set fd [open_cmd_pipe $textconv $path]
483483
} else {
484-
set fd [open $path r]
484+
set fd [safe_open_file $path r]
485485
}
486486
fconfigure $fd -eofchar {}
487487
} else {
488488
if {$do_textconv ne 0} {
489-
set fd [git_read cat-file --textconv "$commit:$path"]
489+
set fd [git_read [list cat-file --textconv "$commit:$path"]]
490490
} else {
491-
set fd [git_read cat-file blob "$commit:$path"]
491+
set fd [git_read [list cat-file blob "$commit:$path"]]
492492
}
493493
}
494494
fconfigure $fd \
@@ -617,7 +617,7 @@ method _exec_blame {cur_w cur_d options cur_s} {
617617
}
618618

619619
lappend options -- $path
620-
set fd [eval git_read --nice blame $options]
620+
set fd [git_read_nice [concat blame $options]]
621621
fconfigure $fd -blocking 0 -translation lf -encoding utf-8
622622
fileevent $fd readable [cb _read_blame $fd $cur_w $cur_d]
623623
set current_fd $fd
@@ -986,7 +986,7 @@ method _showcommit {cur_w lno} {
986986
if {[catch {set msg $header($cmit,message)}]} {
987987
set msg {}
988988
catch {
989-
set fd [git_read cat-file commit $cmit]
989+
set fd [git_read [list cat-file commit $cmit]]
990990
fconfigure $fd -encoding binary -translation lf
991991
# By default commits are assumed to be in utf-8
992992
set enc utf-8
@@ -1134,7 +1134,7 @@ method _blameparent {} {
11341134
} else {
11351135
set diffcmd [list diff-tree --unified=0 $cparent $cmit -- $new_path]
11361136
}
1137-
if {[catch {set fd [eval git_read $diffcmd]} err]} {
1137+
if {[catch {set fd [git_read $diffcmd]} err]} {
11381138
$status_operation stop [mc "Unable to display parent"]
11391139
error_popup [strcat [mc "Error loading diff:"] "\n\n$err"]
11401140
return

lib/branch.tcl

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ proc load_all_heads {} {
77
set rh refs/heads
88
set rh_len [expr {[string length $rh] + 1}]
99
set all_heads [list]
10-
set fd [git_read for-each-ref --format=%(refname) $rh]
10+
set fd [git_read [list for-each-ref --format=%(refname) $rh]]
1111
fconfigure $fd -translation binary -encoding utf-8
1212
while {[gets $fd line] > 0} {
1313
if {!$some_heads_tracking || ![is_tracking_branch $line]} {
@@ -21,10 +21,10 @@ proc load_all_heads {} {
2121

2222
proc load_all_tags {} {
2323
set all_tags [list]
24-
set fd [git_read for-each-ref \
24+
set fd [git_read [list for-each-ref \
2525
--sort=-taggerdate \
2626
--format=%(refname) \
27-
refs/tags]
27+
refs/tags]]
2828
fconfigure $fd -translation binary -encoding utf-8
2929
while {[gets $fd line] > 0} {
3030
if {![regsub ^refs/tags/ $line {} name]} continue

lib/browser.tcl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ method _ls {tree_id {name {}}} {
196196
lappend browser_stack [list $tree_id $name]
197197
$w conf -state disabled
198198

199-
set fd [git_read ls-tree -z $tree_id]
199+
set fd [git_read [list ls-tree -z $tree_id]]
200200
fconfigure $fd -blocking 0 -translation binary -encoding utf-8
201201
fileevent $fd readable [cb _read $fd]
202202
}

lib/checkout_op.tcl

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -304,12 +304,12 @@ The rescan will be automatically started now.
304304
_readtree $this
305305
} else {
306306
ui_status [mc "Refreshing file status..."]
307-
set fd [git_read update-index \
307+
set fd [git_read [list update-index \
308308
-q \
309309
--unmerged \
310310
--ignore-missing \
311311
--refresh \
312-
]
312+
]]
313313
fconfigure $fd -blocking 0 -translation binary
314314
fileevent $fd readable [cb _refresh_wait $fd]
315315
}
@@ -345,14 +345,15 @@ method _readtree {} {
345345
[mc "Updating working directory to '%s'..." [_name $this]] \
346346
[mc "files checked out"]]
347347

348-
set fd [git_read --stderr read-tree \
348+
set fd [git_read [list read-tree \
349349
-m \
350350
-u \
351351
-v \
352352
--exclude-per-directory=.gitignore \
353353
$HEAD \
354354
$new_hash \
355-
]
355+
] \
356+
[list 2>@1]]
356357
fconfigure $fd -blocking 0 -translation binary
357358
fileevent $fd readable [cb _readtree_wait $fd $status_bar_operation]
358359
}
@@ -510,18 +511,8 @@ method _update_repo_state {} {
510511
delete_this
511512
}
512513

513-
git-version proc _detach_HEAD {log new} {
514-
>= 1.5.3 {
515-
git update-ref --no-deref -m $log HEAD $new
516-
}
517-
default {
518-
set p [gitdir HEAD]
519-
file delete $p
520-
set fd [open $p w]
521-
fconfigure $fd -translation lf -encoding utf-8
522-
puts $fd $new
523-
close $fd
524-
}
514+
proc _detach_HEAD {log new} {
515+
git update-ref --no-deref -m $log HEAD $new
525516
}
526517

527518
method _confirm_reset {cur} {
@@ -582,7 +573,7 @@ method _confirm_reset {cur} {
582573
pack $w.buttons.cancel -side right -padx 5
583574
pack $w.buttons -side bottom -fill x -pady 10 -padx 10
584575

585-
set fd [git_read rev-list --pretty=oneline $cur ^$new_hash]
576+
set fd [git_read [list rev-list --pretty=oneline $cur ^$new_hash]]
586577
while {[gets $fd line] > 0} {
587578
set abbr [string range $line 0 7]
588579
set subj [string range $line 41 end]

lib/choose_repository.tcl

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -641,8 +641,8 @@ method _do_clone2 {} {
641641
set pwd [pwd]
642642
if {[catch {
643643
file mkdir [gitdir objects info]
644-
set f_in [open [file join $objdir info alternates] r]
645-
set f_cp [open [gitdir objects info alternates] w]
644+
set f_in [safe_open_file [file join $objdir info alternates] r]
645+
set f_cp [safe_open_file [gitdir objects info alternates] w]
646646
fconfigure $f_in -translation binary -encoding binary
647647
fconfigure $f_cp -translation binary -encoding binary
648648
cd $objdir
@@ -727,7 +727,7 @@ method _do_clone2 {} {
727727
[cb _do_clone_tags]
728728
}
729729
shared {
730-
set fd [open [gitdir objects info alternates] w]
730+
set fd [safe_open_file [gitdir objects info alternates] w]
731731
fconfigure $fd -translation binary
732732
puts $fd $objdir
733733
close $fd
@@ -760,8 +760,8 @@ method _copy_files {objdir tocopy} {
760760
}
761761
foreach p $tocopy {
762762
if {[catch {
763-
set f_in [open [file join $objdir $p] r]
764-
set f_cp [open [file join .git objects $p] w]
763+
set f_in [safe_open_file [file join $objdir $p] r]
764+
set f_cp [safe_open_file [file join .git objects $p] w]
765765
fconfigure $f_in -translation binary -encoding binary
766766
fconfigure $f_cp -translation binary -encoding binary
767767

@@ -818,12 +818,12 @@ method _clone_refs {} {
818818
error_popup [mc "Not a Git repository: %s" [file tail $origin_url]]
819819
return 0
820820
}
821-
set fd_in [git_read for-each-ref \
821+
set fd_in [git_read [list for-each-ref \
822822
--tcl \
823-
{--format=list %(refname) %(objectname) %(*objectname)}]
823+
{--format=list %(refname) %(objectname) %(*objectname)}]]
824824
cd $pwd
825825

826-
set fd [open [gitdir packed-refs] w]
826+
set fd [safe_open_file [gitdir packed-refs] w]
827827
fconfigure $fd -translation binary
828828
puts $fd "# pack-refs with: peeled"
829829
while {[gets $fd_in line] >= 0} {
@@ -877,7 +877,7 @@ method _do_clone_full_end {ok} {
877877

878878
set HEAD {}
879879
if {[file exists [gitdir FETCH_HEAD]]} {
880-
set fd [open [gitdir FETCH_HEAD] r]
880+
set fd [safe_open_file [gitdir FETCH_HEAD] r]
881881
while {[gets $fd line] >= 0} {
882882
if {[regexp "^(.{40})\t\t" $line line HEAD]} {
883883
break
@@ -953,13 +953,14 @@ method _do_clone_checkout {HEAD} {
953953
[mc "files"]]
954954

955955
set readtree_err {}
956-
set fd [git_read --stderr read-tree \
956+
set fd [git_read [list read-tree \
957957
-m \
958958
-u \
959959
-v \
960960
HEAD \
961961
HEAD \
962-
]
962+
] \
963+
[list 2>@1]]
963964
fconfigure $fd -blocking 0 -translation binary
964965
fileevent $fd readable [cb _readtree_wait $fd]
965966
}

lib/choose_rev.tcl

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -146,14 +146,14 @@ constructor _new {path unmerged_only title} {
146146
append fmt { %(*subject)}
147147
append fmt {]}
148148
set all_refn [list]
149-
set fr_fd [git_read for-each-ref \
149+
set fr_fd [git_read [list for-each-ref \
150150
--tcl \
151151
--sort=-taggerdate \
152152
--format=$fmt \
153153
refs/heads \
154154
refs/remotes \
155155
refs/tags \
156-
]
156+
]]
157157
fconfigure $fr_fd -translation lf -encoding utf-8
158158
while {[gets $fr_fd line] > 0} {
159159
set line [eval $line]
@@ -176,7 +176,7 @@ constructor _new {path unmerged_only title} {
176176
close $fr_fd
177177

178178
if {$unmerged_only} {
179-
set fr_fd [git_read rev-list --all ^$::HEAD]
179+
set fr_fd [git_read [list rev-list --all ^$::HEAD]]
180180
while {[gets $fr_fd sha1] > 0} {
181181
if {[catch {set rlst $cmt_refn($sha1)}]} continue
182182
foreach refn $rlst {
@@ -579,7 +579,7 @@ method _reflog_last {name} {
579579

580580
set last {}
581581
if {[catch {set last [file mtime [gitdir $name]]}]
582-
&& ![catch {set g [open [gitdir logs $name] r]}]} {
582+
&& ![catch {set g [safe_open_file [gitdir logs $name] r]}]} {
583583
fconfigure $g -translation binary
584584
while {[gets $g line] >= 0} {
585585
if {[regexp {> ([1-9][0-9]*) } $line line when]} {

lib/commit.tcl

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ You are currently in the middle of a merge that has not been fully completed. Y
2727
if {[catch {
2828
set name ""
2929
set email ""
30-
set fd [git_read cat-file commit $curHEAD]
30+
set fd [git_read [list cat-file commit $curHEAD]]
3131
fconfigure $fd -encoding binary -translation lf
3232
# By default commits are assumed to be in utf-8
3333
set enc utf-8
@@ -234,7 +234,7 @@ A good commit message has the following format:
234234
# -- Build the message file.
235235
#
236236
set msg_p [gitdir GITGUI_EDITMSG]
237-
set msg_wt [open $msg_p w]
237+
set msg_wt [safe_open_file $msg_p w]
238238
fconfigure $msg_wt -translation lf
239239
setup_commit_encoding $msg_wt
240240
puts $msg_wt $msg
@@ -334,7 +334,7 @@ proc commit_commitmsg_wait {fd_ph curHEAD msg_p} {
334334

335335
proc commit_writetree {curHEAD msg_p} {
336336
ui_status [mc "Committing changes..."]
337-
set fd_wt [git_read write-tree]
337+
set fd_wt [git_read [list write-tree]]
338338
fileevent $fd_wt readable \
339339
[list commit_committree $fd_wt $curHEAD $msg_p]
340340
}
@@ -359,7 +359,7 @@ proc commit_committree {fd_wt curHEAD msg_p} {
359359
# -- Verify this wasn't an empty change.
360360
#
361361
if {$commit_type eq {normal}} {
362-
set fd_ot [git_read cat-file commit $PARENT]
362+
set fd_ot [git_read [list cat-file commit $PARENT]]
363363
fconfigure $fd_ot -encoding binary -translation lf
364364
set old_tree [gets $fd_ot]
365365
close $fd_ot
@@ -397,8 +397,8 @@ A rescan will be automatically started now.
397397
foreach p [concat $PARENT $MERGE_HEAD] {
398398
lappend cmd -p $p
399399
}
400-
lappend cmd <$msg_p
401-
if {[catch {set cmt_id [eval git $cmd]} err]} {
400+
set msgtxt [list <$msg_p]
401+
if {[catch {set cmt_id [git_redir $cmd $msgtxt]} err]} {
402402
catch {file delete $msg_p}
403403
error_popup [strcat [mc "commit-tree failed:"] "\n\n$err"]
404404
ui_status [mc "Commit failed."]
@@ -418,7 +418,7 @@ A rescan will be automatically started now.
418418
if {$commit_type ne {normal}} {
419419
append reflogm " ($commit_type)"
420420
}
421-
set msg_fd [open $msg_p r]
421+
set msg_fd [safe_open_file $msg_p r]
422422
setup_commit_encoding $msg_fd 1
423423
gets $msg_fd subject
424424
close $msg_fd

lib/console.tcl

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,9 @@ method _init {} {
9292

9393
method exec {cmd {after {}}} {
9494
if {[lindex $cmd 0] eq {git}} {
95-
set fd_f [eval git_read --stderr [lrange $cmd 1 end]]
95+
set fd_f [git_read [lrange $cmd 1 end] [list 2>@1]]
9696
} else {
97-
lappend cmd 2>@1
98-
set fd_f [_open_stdout_stderr $cmd]
97+
set fd_f [safe_open_command $cmd [list 2>@1]]
9998
}
10099
fconfigure $fd_f -blocking 0 -translation binary -encoding [encoding system]
101100
fileevent $fd_f readable [cb _read $fd_f $after]

lib/database.tcl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
proc do_stats {} {
55
global use_ttk NS
6-
set fd [git_read count-objects -v]
6+
set fd [git_read [list count-objects -v]]
77
while {[gets $fd line] > 0} {
88
if {[regexp {^([^:]+): (\d+)$} $line _ name value]} {
99
set stats($name) $value

0 commit comments

Comments
 (0)