Skip to content

Commit 1925c29

Browse files
mark987j6t
mark987
authored and
j6t
committed
gitk: override $PATH search only on Windows
Commit 4cbe9e0 was written to address problems that result from Tcl's documented behavior on Windows where the current working directory and a number of Windows system directories are automatically prepended to $PATH when searching for executables [1]. This basic Windows behavior has resulted in more than one CVE against git for Windows: CVE-2023-23618, CVE-2022-41953 are listed on the git for Windows github website for the Tcl components of git (gitk, git-gui). 4cbe9e0 is intended to restrict the search to looking only in directories given in $PATH and in the given order, which is exactly the Tcl behavior documented to exist on non-Windows platforms [1]. Thus, this change could have been written to affect only Windows, leaving other platforms alone. However, 4cbe9e0 implements the override for all platforms. This includes specialized code for Cygwin, copied from git-gui prior to commit 7145c65 on https://github.com/j6t/git-gui, so targets a long retired Cygwin port of the Windows Tcl/Tk using Windows pathnames. Since 2012, Cygwin uses a Unix/X11 port requiring Unix pathnames, meaning 4cbe9e0 is incompatible. 4cbe9e0 also induces an infinite recursion as _which now invokes the exec wrapper that invokes _which. This is part of git v2.49.0, so gitk on Cygwin is broken in that release. Rather than fix the unnecessary override code for Cygwin, let's just limit the override of exec/open to Windows, leaving all other platforms using their native exec/open as they did prior to 4cbe9e0. This patch wraps the override code in an "if {[is_Windows]} { ... }" block while removing the non-Windows code added in 4cbe9e0. [1] see https://www.tcl-lang.org/man/tcl8.6/TclCmd/exec.htm Signed-off-by: Mark Levedahl <[email protected]> Signed-off-by: Johannes Sixt <[email protected]>
1 parent b55e113 commit 1925c29

File tree

1 file changed

+59
-89
lines changed

1 file changed

+59
-89
lines changed

‎gitk

Lines changed: 59 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -13,133 +13,103 @@ package require Tk
1313
##
1414
## Enabling platform-specific code paths
1515

16-
proc is_MacOSX {} {
17-
if {[tk windowingsystem] eq {aqua}} {
18-
return 1
19-
}
20-
return 0
21-
}
22-
2316
proc is_Windows {} {
2417
if {$::tcl_platform(platform) eq {windows}} {
2518
return 1
2619
}
2720
return 0
2821
}
2922

30-
set _iscygwin {}
31-
proc is_Cygwin {} {
32-
global _iscygwin
33-
if {$_iscygwin eq {}} {
34-
if {[string match "CYGWIN_*" $::tcl_platform(os)]} {
35-
set _iscygwin 1
36-
} else {
37-
set _iscygwin 0
38-
}
39-
}
40-
return $_iscygwin
41-
}
42-
4323
######################################################################
4424
##
4525
## PATH lookup
4626

47-
set _search_path {}
48-
proc _which {what args} {
49-
global env _search_exe _search_path
50-
51-
if {$_search_path eq {}} {
52-
if {[is_Cygwin] && [regexp {^(/|\.:)} $env(PATH)]} {
53-
set _search_path [split [exec cygpath \
54-
--windows \
55-
--path \
56-
--absolute \
57-
$env(PATH)] {;}]
58-
set _search_exe .exe
59-
} elseif {[is_Windows]} {
27+
if {[is_Windows]} {
28+
set _search_path {}
29+
proc _which {what args} {
30+
global env _search_exe _search_path
31+
32+
if {$_search_path eq {}} {
6033
set gitguidir [file dirname [info script]]
6134
regsub -all ";" $gitguidir "\\;" gitguidir
6235
set env(PATH) "$gitguidir;$env(PATH)"
6336
set _search_path [split $env(PATH) {;}]
6437
# Skip empty `PATH` elements
6538
set _search_path [lsearch -all -inline -not -exact \
66-
$_search_path ""]
39+
$_search_path ""]
6740
set _search_exe .exe
68-
} else {
69-
set _search_path [split $env(PATH) :]
70-
set _search_exe {}
7141
}
72-
}
7342
74-
if {[is_Windows] && [lsearch -exact $args -script] >= 0} {
75-
set suffix {}
76-
} else {
77-
set suffix $_search_exe
78-
}
43+
if {[lsearch -exact $args -script] >= 0} {
44+
set suffix {}
45+
} else {
46+
set suffix $_search_exe
47+
}
7948
80-
foreach p $_search_path {
81-
set p [file join $p $what$suffix]
82-
if {[file exists $p]} {
83-
return [file normalize $p]
49+
foreach p $_search_path {
50+
set p [file join $p $what$suffix]
51+
if {[file exists $p]} {
52+
return [file normalize $p]
53+
}
8454
}
55+
return {}
8556
}
86-
return {}
87-
}
8857
89-
proc sanitize_command_line {command_line from_index} {
90-
set i $from_index
91-
while {$i < [llength $command_line]} {
92-
set cmd [lindex $command_line $i]
93-
if {[file pathtype $cmd] ne "absolute"} {
94-
set fullpath [_which $cmd]
95-
if {$fullpath eq ""} {
96-
throw {NOT-FOUND} "$cmd not found in PATH"
58+
proc sanitize_command_line {command_line from_index} {
59+
set i $from_index
60+
while {$i < [llength $command_line]} {
61+
set cmd [lindex $command_line $i]
62+
if {[file pathtype $cmd] ne "absolute"} {
63+
set fullpath [_which $cmd]
64+
if {$fullpath eq ""} {
65+
throw {NOT-FOUND} "$cmd not found in PATH"
66+
}
67+
lset command_line $i $fullpath
9768
}
98-
lset command_line $i $fullpath
99-
}
10069
101-
# handle piped commands, e.g. `exec A | B`
102-
for {incr i} {$i < [llength $command_line]} {incr i} {
103-
if {[lindex $command_line $i] eq "|"} {
104-
incr i
105-
break
70+
# handle piped commands, e.g. `exec A | B`
71+
for {incr i} {$i < [llength $command_line]} {incr i} {
72+
if {[lindex $command_line $i] eq "|"} {
73+
incr i
74+
break
75+
}
10676
}
10777
}
78+
return $command_line
10879
}
109-
return $command_line
110-
}
11180
112-
# Override `exec` to avoid unsafe PATH lookup
81+
# Override `exec` to avoid unsafe PATH lookup
11382
114-
rename exec real_exec
83+
rename exec real_exec
11584
116-
proc exec {args} {
117-
# skip options
118-
for {set i 0} {$i < [llength $args]} {incr i} {
119-
set arg [lindex $args $i]
120-
if {$arg eq "--"} {
121-
incr i
122-
break
123-
}
124-
if {[string range $arg 0 0] ne "-"} {
125-
break
85+
proc exec {args} {
86+
# skip options
87+
for {set i 0} {$i < [llength $args]} {incr i} {
88+
set arg [lindex $args $i]
89+
if {$arg eq "--"} {
90+
incr i
91+
break
92+
}
93+
if {[string range $arg 0 0] ne "-"} {
94+
break
95+
}
12696
}
97+
set args [sanitize_command_line $args $i]
98+
uplevel 1 real_exec $args
12799
}
128-
set args [sanitize_command_line $args $i]
129-
uplevel 1 real_exec $args
130-
}
131100
132-
# Override `open` to avoid unsafe PATH lookup
101+
# Override `open` to avoid unsafe PATH lookup
133102
134-
rename open real_open
103+
rename open real_open
135104
136-
proc open {args} {
137-
set arg0 [lindex $args 0]
138-
if {[string range $arg0 0 0] eq "|"} {
139-
set command_line [string trim [string range $arg0 1 end]]
140-
lset args 0 "| [sanitize_command_line $command_line 0]"
105+
proc open {args} {
106+
set arg0 [lindex $args 0]
107+
if {[string range $arg0 0 0] eq "|"} {
108+
set command_line [string trim [string range $arg0 1 end]]
109+
lset args 0 "| [sanitize_command_line $command_line 0]"
110+
}
111+
uplevel 1 real_open $args
141112
}
142-
uplevel 1 real_open $args
143113
}
144114
145115
# End of safe PATH lookup stuff

0 commit comments

Comments
 (0)