!92 Fix CVE-2022-41953
From: @fly_fzc Reviewed-by: @xujing99 Signed-off-by: @xujing99
This commit is contained in:
openeuler-ci-bot
Gitee
commit
be06bb7e3b
@ -0,0 +1,101 @@
|
|||||||
|
From 15ea1198bae31e248e01261b0163d024c0351695 Mon Sep 17 00:00:00 2001
|
||||||
|
From: Johannes Schindelin <johannes.schindelin@gmx.de>
|
||||||
|
Date: Fri, 16 Dec 2022 20:41:28 +0100
|
||||||
|
Subject: [PATCH] Move is_<platform> functions to the beginning
|
||||||
|
|
||||||
|
We need these in `_which` and they should be defined before that
|
||||||
|
function's definition.
|
||||||
|
|
||||||
|
This commit is best viewed with `--color-moved`.
|
||||||
|
|
||||||
|
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
|
||||||
|
---
|
||||||
|
git-gui/git-gui.sh | 58 +++++++++++++++++++++++++---------------------
|
||||||
|
1 file changed, 31 insertions(+), 27 deletions(-)
|
||||||
|
|
||||||
|
diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
|
||||||
|
index 0fe60f80cc..f779fc9268 100755
|
||||||
|
--- a/git-gui/git-gui.sh
|
||||||
|
+++ b/git-gui/git-gui.sh
|
||||||
|
@@ -44,6 +44,37 @@ if {[catch {package require Tcl 8.5} err]
|
||||||
|
|
||||||
|
catch {rename send {}} ; # What an evil concept...
|
||||||
|
|
||||||
|
+######################################################################
|
||||||
|
+##
|
||||||
|
+## Enabling platform-specific code paths
|
||||||
|
+
|
||||||
|
+proc is_MacOSX {} {
|
||||||
|
+ if {[tk windowingsystem] eq {aqua}} {
|
||||||
|
+ return 1
|
||||||
|
+ }
|
||||||
|
+ return 0
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
+proc is_Windows {} {
|
||||||
|
+ if {$::tcl_platform(platform) eq {windows}} {
|
||||||
|
+ return 1
|
||||||
|
+ }
|
||||||
|
+ return 0
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
+set _iscygwin {}
|
||||||
|
+proc is_Cygwin {} {
|
||||||
|
+ global _iscygwin
|
||||||
|
+ if {$_iscygwin eq {}} {
|
||||||
|
+ if {[string match "CYGWIN_*" $::tcl_platform(os)]} {
|
||||||
|
+ set _iscygwin 1
|
||||||
|
+ } else {
|
||||||
|
+ set _iscygwin 0
|
||||||
|
+ }
|
||||||
|
+ }
|
||||||
|
+ return $_iscygwin
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
######################################################################
|
||||||
|
##
|
||||||
|
## locate our library
|
||||||
|
@@ -163,7 +194,6 @@ set _isbare {}
|
||||||
|
set _gitexec {}
|
||||||
|
set _githtmldir {}
|
||||||
|
set _reponame {}
|
||||||
|
-set _iscygwin {}
|
||||||
|
set _search_path {}
|
||||||
|
set _shellpath {@@SHELL_PATH@@}
|
||||||
|
|
||||||
|
@@ -252,32 +282,6 @@ proc reponame {} {
|
||||||
|
return $::_reponame
|
||||||
|
}
|
||||||
|
|
||||||
|
-proc is_MacOSX {} {
|
||||||
|
- if {[tk windowingsystem] eq {aqua}} {
|
||||||
|
- return 1
|
||||||
|
- }
|
||||||
|
- return 0
|
||||||
|
-}
|
||||||
|
-
|
||||||
|
-proc is_Windows {} {
|
||||||
|
- if {$::tcl_platform(platform) eq {windows}} {
|
||||||
|
- return 1
|
||||||
|
- }
|
||||||
|
- return 0
|
||||||
|
-}
|
||||||
|
-
|
||||||
|
-proc is_Cygwin {} {
|
||||||
|
- global _iscygwin
|
||||||
|
- if {$_iscygwin eq {}} {
|
||||||
|
- if {[string match "CYGWIN_*" $::tcl_platform(os)]} {
|
||||||
|
- set _iscygwin 1
|
||||||
|
- } else {
|
||||||
|
- set _iscygwin 0
|
||||||
|
- }
|
||||||
|
- }
|
||||||
|
- return $_iscygwin
|
||||||
|
-}
|
||||||
|
-
|
||||||
|
proc is_enabled {option} {
|
||||||
|
global enabled_options
|
||||||
|
if {[catch {set on $enabled_options($option)}]} {return 0}
|
||||||
|
--
|
||||||
|
2.27.0
|
||||||
|
|
||||||
@ -0,0 +1,135 @@
|
|||||||
|
From 24f3f5833430d814f2c62220494741ea3d8cf4b3 Mon Sep 17 00:00:00 2001
|
||||||
|
From: Johannes Schindelin <johannes.schindelin@gmx.de>
|
||||||
|
Date: Mon, 5 Dec 2022 14:37:41 +0100
|
||||||
|
Subject: [PATCH] Move the `_which` function (almost) to the top
|
||||||
|
|
||||||
|
We are about to make use of the `_which` function to address
|
||||||
|
CVE-2022-41953 by overriding Tcl/Tk's unsafe PATH lookup on Windows.
|
||||||
|
|
||||||
|
In preparation for that, let's move it close to the top of the file to
|
||||||
|
make sure that even early `exec` calls that happen during the start-up
|
||||||
|
of Git GUI benefit from the fix.
|
||||||
|
|
||||||
|
This commit is best viewed with `--color-moved`.
|
||||||
|
|
||||||
|
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
|
||||||
|
---
|
||||||
|
git-gui/git-gui.sh | 88 ++++++++++++++++++++++++----------------------
|
||||||
|
1 file changed, 46 insertions(+), 42 deletions(-)
|
||||||
|
|
||||||
|
diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
|
||||||
|
index f779fc9268..b0eb5a6ae4 100755
|
||||||
|
--- a/git-gui/git-gui.sh
|
||||||
|
+++ b/git-gui/git-gui.sh
|
||||||
|
@@ -75,6 +75,52 @@ proc is_Cygwin {} {
|
||||||
|
return $_iscygwin
|
||||||
|
}
|
||||||
|
|
||||||
|
+######################################################################
|
||||||
|
+##
|
||||||
|
+## PATH lookup
|
||||||
|
+
|
||||||
|
+set _search_path {}
|
||||||
|
+proc _which {what args} {
|
||||||
|
+ global env _search_exe _search_path
|
||||||
|
+
|
||||||
|
+ if {$_search_path eq {}} {
|
||||||
|
+ if {[is_Cygwin] && [regexp {^(/|\.:)} $env(PATH)]} {
|
||||||
|
+ set _search_path [split [exec cygpath \
|
||||||
|
+ --windows \
|
||||||
|
+ --path \
|
||||||
|
+ --absolute \
|
||||||
|
+ $env(PATH)] {;}]
|
||||||
|
+ set _search_exe .exe
|
||||||
|
+ } elseif {[is_Windows]} {
|
||||||
|
+ set gitguidir [file dirname [info script]]
|
||||||
|
+ regsub -all ";" $gitguidir "\\;" gitguidir
|
||||||
|
+ set env(PATH) "$gitguidir;$env(PATH)"
|
||||||
|
+ set _search_path [split $env(PATH) {;}]
|
||||||
|
+ # Skip empty `PATH` elements
|
||||||
|
+ set _search_path [lsearch -all -inline -not -exact \
|
||||||
|
+ $_search_path ""]
|
||||||
|
+ set _search_exe .exe
|
||||||
|
+ } else {
|
||||||
|
+ set _search_path [split $env(PATH) :]
|
||||||
|
+ set _search_exe {}
|
||||||
|
+ }
|
||||||
|
+ }
|
||||||
|
+
|
||||||
|
+ if {[is_Windows] && [lsearch -exact $args -script] >= 0} {
|
||||||
|
+ 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]
|
||||||
|
+ }
|
||||||
|
+ }
|
||||||
|
+ return {}
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
######################################################################
|
||||||
|
##
|
||||||
|
## locate our library
|
||||||
|
@@ -194,7 +240,6 @@ set _isbare {}
|
||||||
|
set _gitexec {}
|
||||||
|
set _githtmldir {}
|
||||||
|
set _reponame {}
|
||||||
|
-set _search_path {}
|
||||||
|
set _shellpath {@@SHELL_PATH@@}
|
||||||
|
|
||||||
|
set _trace [lsearch -exact $argv --trace]
|
||||||
|
@@ -444,47 +489,6 @@ proc _git_cmd {name} {
|
||||||
|
return $v
|
||||||
|
}
|
||||||
|
|
||||||
|
-proc _which {what args} {
|
||||||
|
- global env _search_exe _search_path
|
||||||
|
-
|
||||||
|
- if {$_search_path eq {}} {
|
||||||
|
- if {[is_Cygwin] && [regexp {^(/|\.:)} $env(PATH)]} {
|
||||||
|
- set _search_path [split [exec cygpath \
|
||||||
|
- --windows \
|
||||||
|
- --path \
|
||||||
|
- --absolute \
|
||||||
|
- $env(PATH)] {;}]
|
||||||
|
- set _search_exe .exe
|
||||||
|
- } elseif {[is_Windows]} {
|
||||||
|
- set gitguidir [file dirname [info script]]
|
||||||
|
- regsub -all ";" $gitguidir "\\;" gitguidir
|
||||||
|
- set env(PATH) "$gitguidir;$env(PATH)"
|
||||||
|
- set _search_path [split $env(PATH) {;}]
|
||||||
|
- # Skip empty `PATH` elements
|
||||||
|
- set _search_path [lsearch -all -inline -not -exact \
|
||||||
|
- $_search_path ""]
|
||||||
|
- set _search_exe .exe
|
||||||
|
- } else {
|
||||||
|
- set _search_path [split $env(PATH) :]
|
||||||
|
- set _search_exe {}
|
||||||
|
- }
|
||||||
|
- }
|
||||||
|
-
|
||||||
|
- if {[is_Windows] && [lsearch -exact $args -script] >= 0} {
|
||||||
|
- 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]
|
||||||
|
- }
|
||||||
|
- }
|
||||||
|
- return {}
|
||||||
|
-}
|
||||||
|
-
|
||||||
|
# Test a file for a hashbang to identify executable scripts on Windows.
|
||||||
|
proc is_shellscript {filename} {
|
||||||
|
if {![file exists $filename]} {return 0}
|
||||||
|
--
|
||||||
|
2.27.0
|
||||||
|
|
||||||
@ -0,0 +1,125 @@
|
|||||||
|
From 2dd84542702a038496a23af4da8ad8059d0da91f Mon Sep 17 00:00:00 2001
|
||||||
|
From: Johannes Schindelin <johannes.schindelin@gmx.de>
|
||||||
|
Date: Wed, 23 Nov 2022 09:31:06 +0100
|
||||||
|
Subject: [PATCH] Work around Tcl's default `PATH` lookup
|
||||||
|
|
||||||
|
As per https://www.tcl.tk/man/tcl8.6/TclCmd/exec.html#M23, Tcl's `exec`
|
||||||
|
function goes out of its way to imitate the highly dangerous path lookup
|
||||||
|
of `cmd.exe`, but _of course_ only on Windows:
|
||||||
|
|
||||||
|
If a directory name was not specified as part of the application
|
||||||
|
name, the following directories are automatically searched in
|
||||||
|
order when attempting to locate the application:
|
||||||
|
|
||||||
|
The directory from which the Tcl executable was loaded.
|
||||||
|
|
||||||
|
The current directory.
|
||||||
|
|
||||||
|
The Windows 32-bit system directory.
|
||||||
|
|
||||||
|
The Windows home directory.
|
||||||
|
|
||||||
|
The directories listed in the path.
|
||||||
|
|
||||||
|
The dangerous part is the second item, of course: `exec` _prefers_
|
||||||
|
executables in the current directory to those that are actually in the
|
||||||
|
`PATH`.
|
||||||
|
|
||||||
|
It is almost as if people wanted to Windows users vulnerable,
|
||||||
|
specifically.
|
||||||
|
|
||||||
|
To avoid that, Git GUI already has the `_which` function that does not
|
||||||
|
imitate that dangerous practice when looking up executables in the
|
||||||
|
search path.
|
||||||
|
|
||||||
|
However, Git GUI currently fails to use that function e.g. when trying to
|
||||||
|
execute `aspell` for spell checking.
|
||||||
|
|
||||||
|
That is not only dangerous but combined with Tcl's unfortunate default
|
||||||
|
behavior and with the fact that Git GUI tries to spell-check a
|
||||||
|
repository just after cloning, leads to a critical Remote Code Execution
|
||||||
|
vulnerability.
|
||||||
|
|
||||||
|
Let's override both `exec` and `open` to always use `_which` instead of
|
||||||
|
letting Tcl perform the path lookup, to prevent this attack vector.
|
||||||
|
|
||||||
|
This addresses CVE-2022-41953.
|
||||||
|
|
||||||
|
For more details, see
|
||||||
|
https://github.com/git-for-windows/git/security/advisories/GHSA-v4px-mx59-w99c
|
||||||
|
|
||||||
|
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
|
||||||
|
---
|
||||||
|
git-gui/git-gui.sh | 56 ++++++++++++++++++++++++++++++++++++++++++++++
|
||||||
|
1 file changed, 56 insertions(+)
|
||||||
|
|
||||||
|
diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
|
||||||
|
index b0eb5a6ae4..cb92bba1c4 100755
|
||||||
|
--- a/git-gui/git-gui.sh
|
||||||
|
+++ b/git-gui/git-gui.sh
|
||||||
|
@@ -121,6 +121,62 @@ proc _which {what args} {
|
||||||
|
return {}
|
||||||
|
}
|
||||||
|
|
||||||
|
+proc sanitize_command_line {command_line from_index} {
|
||||||
|
+ set i $from_index
|
||||||
|
+ while {$i < [llength $command_line]} {
|
||||||
|
+ set cmd [lindex $command_line $i]
|
||||||
|
+ if {[file pathtype $cmd] ne "absolute"} {
|
||||||
|
+ set fullpath [_which $cmd]
|
||||||
|
+ if {$fullpath eq ""} {
|
||||||
|
+ throw {NOT-FOUND} "$cmd not found in PATH"
|
||||||
|
+ }
|
||||||
|
+ 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
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
+# Override `open` to avoid unsafe PATH lookup
|
||||||
|
+
|
||||||
|
+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]"
|
||||||
|
+ }
|
||||||
|
+ uplevel 1 real_open $args
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
######################################################################
|
||||||
|
##
|
||||||
|
## locate our library
|
||||||
|
--
|
||||||
|
2.27.0
|
||||||
|
|
||||||
@ -0,0 +1,61 @@
|
|||||||
|
From 9121f5c92c781f6e6415b17014c8c6ac864d2e70 Mon Sep 17 00:00:00 2001
|
||||||
|
From: Johannes Schindelin <johannes.schindelin@gmx.de>
|
||||||
|
Date: Sun, 4 Dec 2022 22:56:08 +0100
|
||||||
|
Subject: [PATCH] is_Cygwin: avoid `exec`ing anything
|
||||||
|
|
||||||
|
The `is_Cygwin` function is used, among other things, to determine
|
||||||
|
how executables are discovered in the `PATH` list by the `_which` function.
|
||||||
|
|
||||||
|
We are about to change the behavior of the `_which` function on Windows
|
||||||
|
(but not Cygwin): On Windows, we want it to ignore empty elements of the
|
||||||
|
`PATH` instead of treating them as referring to the current directory
|
||||||
|
(which is a "legacy feature" according to
|
||||||
|
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03,
|
||||||
|
but apparently not explicitly deprecated, the POSIX documentation is
|
||||||
|
quite unclear on that even if the Cygwin project itself considers it to
|
||||||
|
be deprecated: https://github.com/cygwin/cygwin/commit/fc74dbf22f5c).
|
||||||
|
|
||||||
|
This is important because on Windows, `exec` does something very unsafe
|
||||||
|
by default (unless we're running a Cygwin version of Tcl, which follows
|
||||||
|
Unix semantics).
|
||||||
|
|
||||||
|
However, we try to `exec` something _inside_ `is_Cygwin` to determine
|
||||||
|
whether we're running within Cygwin or not, i.e. before we determined
|
||||||
|
whether we need to handle `PATH` specially or not. That's a Catch-22.
|
||||||
|
|
||||||
|
Therefore, and because it is much cleaner anyway, use the
|
||||||
|
`$::tcl_platform(os)` value which is guaranteed to start with `CYGWIN_`
|
||||||
|
when running a Cygwin variant of Tcl/Tk, instead of executing `cygpath
|
||||||
|
--windir`.
|
||||||
|
|
||||||
|
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
|
||||||
|
---
|
||||||
|
git-gui/git-gui.sh | 12 ++----------
|
||||||
|
1 file changed, 2 insertions(+), 10 deletions(-)
|
||||||
|
|
||||||
|
diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
|
||||||
|
index 0cf625ca01..0fe60f80cc 100755
|
||||||
|
--- a/git-gui/git-gui.sh
|
||||||
|
+++ b/git-gui/git-gui.sh
|
||||||
|
@@ -269,16 +269,8 @@ proc is_Windows {} {
|
||||||
|
proc is_Cygwin {} {
|
||||||
|
global _iscygwin
|
||||||
|
if {$_iscygwin eq {}} {
|
||||||
|
- if {$::tcl_platform(platform) eq {windows}} {
|
||||||
|
- if {[catch {set p [exec cygpath --windir]} err]} {
|
||||||
|
- set _iscygwin 0
|
||||||
|
- } else {
|
||||||
|
- set _iscygwin 1
|
||||||
|
- # Handle MSys2 which is only cygwin when MSYSTEM is MSYS.
|
||||||
|
- if {[info exists ::env(MSYSTEM)] && $::env(MSYSTEM) ne "MSYS"} {
|
||||||
|
- set _iscygwin 0
|
||||||
|
- }
|
||||||
|
- }
|
||||||
|
+ if {[string match "CYGWIN_*" $::tcl_platform(os)]} {
|
||||||
|
+ set _iscygwin 1
|
||||||
|
} else {
|
||||||
|
set _iscygwin 0
|
||||||
|
}
|
||||||
|
--
|
||||||
|
2.27.0
|
||||||
|
|
||||||
170
backport-CVE-2022-41953-windows-ignore-empty-PATH-elements.patch
Normal file
170
backport-CVE-2022-41953-windows-ignore-empty-PATH-elements.patch
Normal file
@ -0,0 +1,170 @@
|
|||||||
|
From 5ef19b63bf709cf39059bf67d97ab1dd22ef4a59 Mon Sep 17 00:00:00 2001
|
||||||
|
From: Johannes Schindelin <johannes.schindelin@gmx.de>
|
||||||
|
Date: Wed, 23 Nov 2022 09:12:49 +0100
|
||||||
|
Subject: [PATCH] windows: ignore empty `PATH` elements
|
||||||
|
|
||||||
|
When looking up an executable via the `_which` function, Git GUI
|
||||||
|
imitates the `execlp()` strategy where the environment variable `PATH`
|
||||||
|
is interpreted as a list of paths in which to search.
|
||||||
|
|
||||||
|
For historical reasons, stemming from the olden times when it was
|
||||||
|
uncommon to download a lot of files from the internet into the current
|
||||||
|
directory, empty elements in this list are treated as if the current
|
||||||
|
directory had been specified.
|
||||||
|
|
||||||
|
Nowadays, of course, this treatment is highly dangerous as the current
|
||||||
|
directory often contains files that have just been downloaded and not
|
||||||
|
yet been inspected by the user. Unix/Linux users are essentially
|
||||||
|
expected to be very, very careful to simply not add empty `PATH`
|
||||||
|
elements, i.e. not to make use of that feature.
|
||||||
|
|
||||||
|
On Windows, however, it is quite common for `PATH` to contain empty
|
||||||
|
elements by mistake, e.g. as an unintended left-over entry when an
|
||||||
|
application was installed from the Windows Store and then uninstalled
|
||||||
|
manually.
|
||||||
|
|
||||||
|
While it would probably make most sense to safe-guard not only Windows
|
||||||
|
users, it seems to be common practice to ignore these empty `PATH`
|
||||||
|
elements _only_ on Windows, but not on other platforms.
|
||||||
|
|
||||||
|
Sadly, this practice is followed inconsistently between different
|
||||||
|
software projects, where projects with few, if any, Windows-based
|
||||||
|
contributors tend to be less consistent or even "blissful" about it.
|
||||||
|
Here is a non-exhaustive list:
|
||||||
|
|
||||||
|
Cygwin:
|
||||||
|
|
||||||
|
It specifically "eats" empty paths when converting path lists to
|
||||||
|
POSIX: https://github.com/cygwin/cygwin/commit/753702223c7d
|
||||||
|
|
||||||
|
I.e. it follows the common practice.
|
||||||
|
|
||||||
|
PowerShell:
|
||||||
|
|
||||||
|
It specifically ignores empty paths when searching the `PATH`.
|
||||||
|
The reason for this is apparently so self-evident that it is not
|
||||||
|
even mentioned here:
|
||||||
|
https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_environment_variables#path-information
|
||||||
|
|
||||||
|
I.e. it follows the common practice.
|
||||||
|
|
||||||
|
CMD:
|
||||||
|
|
||||||
|
Oh my, CMD. Let's just forget about it, nobody in their right
|
||||||
|
(security) mind takes CMD as inspiration. It is so unsafe by
|
||||||
|
default that we even planned on dropping `Git CMD` from Git for
|
||||||
|
Windows altogether, and only walked back on that plan when we
|
||||||
|
found a super ugly hack, just to keep Git's users secure by
|
||||||
|
default:
|
||||||
|
|
||||||
|
https://github.com/git-for-windows/MINGW-packages/commit/82172388bb51
|
||||||
|
|
||||||
|
So CMD chooses to hide behind the battle cry "Works as
|
||||||
|
Designed!" that all too often leaves users vulnerable. CMD is
|
||||||
|
probably the most prominent project whose lead you want to avoid
|
||||||
|
following in matters of security.
|
||||||
|
|
||||||
|
Win32 API (`CreateProcess()`)
|
||||||
|
|
||||||
|
Just like CMD, `CreateProcess()` adheres to the original design
|
||||||
|
of the path lookup in the name of backward compatibility (see
|
||||||
|
https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw
|
||||||
|
for details):
|
||||||
|
|
||||||
|
If the file name does not contain a directory path, the
|
||||||
|
system searches for the executable file in the following
|
||||||
|
sequence:
|
||||||
|
|
||||||
|
1. The directory from which the application loaded.
|
||||||
|
|
||||||
|
2. The current directory for the parent process.
|
||||||
|
|
||||||
|
[...]
|
||||||
|
|
||||||
|
I.e. the Win32 API itself chooses backwards compatibility over
|
||||||
|
users' safety.
|
||||||
|
|
||||||
|
Git LFS:
|
||||||
|
|
||||||
|
There have been not one, not two, but three security advisories
|
||||||
|
about Git LFS executing executables from the current directory by
|
||||||
|
mistake. As part of one of them, a change was introduced to stop
|
||||||
|
treating empty `PATH` elements as equivalent to `.`:
|
||||||
|
https://github.com/git-lfs/git-lfs/commit/7cd7bb0a1f0d
|
||||||
|
|
||||||
|
I.e. it follows the common practice.
|
||||||
|
|
||||||
|
Go:
|
||||||
|
|
||||||
|
Go does not follow the common practice, and you can think about
|
||||||
|
that what you want:
|
||||||
|
https://github.com/golang/go/blob/go1.19.3/src/os/exec/lp_windows.go#L114-L135
|
||||||
|
https://github.com/golang/go/blob/go1.19.3/src/path/filepath/path_windows.go#L108-L137
|
||||||
|
|
||||||
|
Git Credential Manager:
|
||||||
|
|
||||||
|
It tries to imitate Git LFS, but unfortunately misses the empty
|
||||||
|
`PATH` element handling. As of time of writing, this is in the
|
||||||
|
process of being fixed:
|
||||||
|
https://github.com/GitCredentialManager/git-credential-manager/pull/968
|
||||||
|
|
||||||
|
So now that we have established that it is a common practice to ignore
|
||||||
|
empty `PATH` elements on Windows, let's assess this commit's change
|
||||||
|
using Schneier's Five-Step Process
|
||||||
|
(https://www.schneier.com/crypto-gram/archives/2002/0415.html#1):
|
||||||
|
|
||||||
|
Step 1: What problem does it solve?
|
||||||
|
|
||||||
|
It prevents an entire class of Remote Code Execution exploits via
|
||||||
|
Git GUI's `Clone` functionality.
|
||||||
|
|
||||||
|
Step 2: How well does it solve that problem?
|
||||||
|
|
||||||
|
Very well. It prevents the attack vector of luring an unsuspecting
|
||||||
|
victim into cloning an executable into the worktree root directory
|
||||||
|
that Git GUI immediately executes.
|
||||||
|
|
||||||
|
Step 3: What other security problems does it cause?
|
||||||
|
|
||||||
|
Maybe non-security problems: If a project (ab-)uses the unsafe
|
||||||
|
`PATH` lookup. That would not only be unsafe, though, but
|
||||||
|
fragile in the first place because it would break when running
|
||||||
|
in a subdirectory. Therefore I would consider this a scenario
|
||||||
|
not worth keeping working.
|
||||||
|
|
||||||
|
Step 4: What are the costs of this measure?
|
||||||
|
|
||||||
|
Almost nil, except for the time writing up this commit message
|
||||||
|
;-)
|
||||||
|
|
||||||
|
Step 5: Given the answers to steps two through four, is the security
|
||||||
|
measure worth the costs?
|
||||||
|
|
||||||
|
Yes. Keeping Git's users Secure By Default is worth it. It's a
|
||||||
|
tiny price to pay compared to the damages even a single
|
||||||
|
successful exploit can cost.
|
||||||
|
|
||||||
|
So let's follow that common practice in Git GUI, too.
|
||||||
|
|
||||||
|
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
|
||||||
|
---
|
||||||
|
git-gui/git-gui.sh | 3 +++
|
||||||
|
1 file changed, 3 insertions(+)
|
||||||
|
|
||||||
|
diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
|
||||||
|
index 201524c34e..0cf625ca01 100755
|
||||||
|
--- a/git-gui/git-gui.sh
|
||||||
|
+++ b/git-gui/git-gui.sh
|
||||||
|
@@ -464,6 +464,9 @@ proc _which {what args} {
|
||||||
|
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 _search_exe .exe
|
||||||
|
} else {
|
||||||
|
set _search_path [split $env(PATH) :]
|
||||||
|
--
|
||||||
|
2.27.0
|
||||||
|
|
||||||
14
git.spec
14
git.spec
@ -1,7 +1,7 @@
|
|||||||
%global gitexecdir %{_libexecdir}/git-core
|
%global gitexecdir %{_libexecdir}/git-core
|
||||||
Name: git
|
Name: git
|
||||||
Version: 2.39.1
|
Version: 2.39.1
|
||||||
Release: 1
|
Release: 2
|
||||||
Summary: A popular and widely used Version Control System
|
Summary: A popular and widely used Version Control System
|
||||||
License: GPLv2+ or LGPLv2.1
|
License: GPLv2+ or LGPLv2.1
|
||||||
URL: https://git-scm.com/
|
URL: https://git-scm.com/
|
||||||
@ -12,6 +12,12 @@ Source100: git-gui.desktop
|
|||||||
Source101: git@.service.in
|
Source101: git@.service.in
|
||||||
Source102: git.socket
|
Source102: git.socket
|
||||||
|
|
||||||
|
Patch0: backport-CVE-2022-41953-windows-ignore-empty-PATH-elements.patch
|
||||||
|
Patch1: backport-CVE-2022-41953-is_Cygwin-avoid-exec-ing-anything.patch
|
||||||
|
Patch2: backport-CVE-2022-41953-Move-is_-platform-functions-to-the-beginning.patch
|
||||||
|
Patch3: backport-CVE-2022-41953-Move-the-_which-function-almost-to-the-top.patch
|
||||||
|
Patch4: backport-CVE-2022-41953-Work-around-Tcl-s-default-PATH-lookup.patch
|
||||||
|
|
||||||
BuildRequires: gcc gettext
|
BuildRequires: gcc gettext
|
||||||
BuildRequires: openssl-devel libcurl-devel expat-devel systemd asciidoc xmlto glib2-devel libsecret-devel pcre-devel desktop-file-utils
|
BuildRequires: openssl-devel libcurl-devel expat-devel systemd asciidoc xmlto glib2-devel libsecret-devel pcre-devel desktop-file-utils
|
||||||
BuildRequires: python3-devel perl-generators perl-interpreter perl-Error perl(Test::More) perl-MailTools perl(Test)
|
BuildRequires: python3-devel perl-generators perl-interpreter perl-Error perl(Test::More) perl-MailTools perl(Test)
|
||||||
@ -292,6 +298,12 @@ make %{?_smp_mflags} test
|
|||||||
%{_mandir}/man7/git*.7.*
|
%{_mandir}/man7/git*.7.*
|
||||||
|
|
||||||
%changelog
|
%changelog
|
||||||
|
* Sat Jan 28 2023 fuanan <fuanan3@h-partners.com> - 2.39.1-2
|
||||||
|
- Type:CVE
|
||||||
|
- ID:CVE-2022-41953
|
||||||
|
- SUG:NA
|
||||||
|
- DESC:Fix CVE-2022-41953
|
||||||
|
|
||||||
* Thu Jan 19 2023 fuanan <fuanan3@h-partners.com> - 2.39.1-1
|
* Thu Jan 19 2023 fuanan <fuanan3@h-partners.com> - 2.39.1-1
|
||||||
- Type:enhancement
|
- Type:enhancement
|
||||||
- ID:NA
|
- ID:NA
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user