From d0fe53ad74bd0ab5d3521dca3b023b0fa67e321e Mon Sep 17 00:00:00 2001 From: fly_fzc <2385803914@qq.com> Date: Sat, 28 Jan 2023 16:32:37 +0800 Subject: [PATCH] Fix CVE-2022-41953 --- ...-platform-functions-to-the-beginning.patch | 101 +++++++++++ ...he-_which-function-almost-to-the-top.patch | 135 ++++++++++++++ ...ork-around-Tcl-s-default-PATH-lookup.patch | 125 +++++++++++++ ...53-is_Cygwin-avoid-exec-ing-anything.patch | 61 +++++++ ...3-windows-ignore-empty-PATH-elements.patch | 170 ++++++++++++++++++ git.spec | 13 +- 6 files changed, 604 insertions(+), 1 deletion(-) create mode 100644 backport-CVE-2022-41953-Move-is_-platform-functions-to-the-beginning.patch create mode 100644 backport-CVE-2022-41953-Move-the-_which-function-almost-to-the-top.patch create mode 100644 backport-CVE-2022-41953-Work-around-Tcl-s-default-PATH-lookup.patch create mode 100644 backport-CVE-2022-41953-is_Cygwin-avoid-exec-ing-anything.patch create mode 100644 backport-CVE-2022-41953-windows-ignore-empty-PATH-elements.patch diff --git a/backport-CVE-2022-41953-Move-is_-platform-functions-to-the-beginning.patch b/backport-CVE-2022-41953-Move-is_-platform-functions-to-the-beginning.patch new file mode 100644 index 0000000..1f385af --- /dev/null +++ b/backport-CVE-2022-41953-Move-is_-platform-functions-to-the-beginning.patch @@ -0,0 +1,101 @@ +From 15ea1198bae31e248e01261b0163d024c0351695 Mon Sep 17 00:00:00 2001 +From: Johannes Schindelin +Date: Fri, 16 Dec 2022 20:41:28 +0100 +Subject: [PATCH] Move is_ 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 +--- + 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 + diff --git a/backport-CVE-2022-41953-Move-the-_which-function-almost-to-the-top.patch b/backport-CVE-2022-41953-Move-the-_which-function-almost-to-the-top.patch new file mode 100644 index 0000000..df85aa7 --- /dev/null +++ b/backport-CVE-2022-41953-Move-the-_which-function-almost-to-the-top.patch @@ -0,0 +1,135 @@ +From 24f3f5833430d814f2c62220494741ea3d8cf4b3 Mon Sep 17 00:00:00 2001 +From: Johannes Schindelin +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 +--- + 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 + diff --git a/backport-CVE-2022-41953-Work-around-Tcl-s-default-PATH-lookup.patch b/backport-CVE-2022-41953-Work-around-Tcl-s-default-PATH-lookup.patch new file mode 100644 index 0000000..464ee42 --- /dev/null +++ b/backport-CVE-2022-41953-Work-around-Tcl-s-default-PATH-lookup.patch @@ -0,0 +1,125 @@ +From 2dd84542702a038496a23af4da8ad8059d0da91f Mon Sep 17 00:00:00 2001 +From: Johannes Schindelin +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 +--- + 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 + diff --git a/backport-CVE-2022-41953-is_Cygwin-avoid-exec-ing-anything.patch b/backport-CVE-2022-41953-is_Cygwin-avoid-exec-ing-anything.patch new file mode 100644 index 0000000..80ee36c --- /dev/null +++ b/backport-CVE-2022-41953-is_Cygwin-avoid-exec-ing-anything.patch @@ -0,0 +1,61 @@ +From 9121f5c92c781f6e6415b17014c8c6ac864d2e70 Mon Sep 17 00:00:00 2001 +From: Johannes Schindelin +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 +--- + 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 + diff --git a/backport-CVE-2022-41953-windows-ignore-empty-PATH-elements.patch b/backport-CVE-2022-41953-windows-ignore-empty-PATH-elements.patch new file mode 100644 index 0000000..2b392be --- /dev/null +++ b/backport-CVE-2022-41953-windows-ignore-empty-PATH-elements.patch @@ -0,0 +1,170 @@ +From 5ef19b63bf709cf39059bf67d97ab1dd22ef4a59 Mon Sep 17 00:00:00 2001 +From: Johannes Schindelin +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 +--- + 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 + diff --git a/git.spec b/git.spec index afaa251..42028f4 100644 --- a/git.spec +++ b/git.spec @@ -1,7 +1,7 @@ %global gitexecdir %{_libexecdir}/git-core Name: git Version: 2.27.0 -Release: 10 +Release: 11 Summary: A popular and widely used Version Control System License: GPLv2+ or LGPLv2.1 URL: https://git-scm.com/ @@ -50,6 +50,11 @@ Patch35: backport-CVE-2022-41903-utf8-fix-overflow-when-returning-string-widt Patch36: backport-CVE-2022-41903-utf8-fix-checking-for-glyph-width-in-strbuf_utf8_rep.patch Patch37: backport-CVE-2022-41903-utf8-refactor-strbuf_utf8_replace-to-not-rely-on-pre.patch Patch38: backport-CVE-2022-41903-pretty-restrict-input-lengths-for-padding-and-wrappi.patch +Patch39: backport-CVE-2022-41953-windows-ignore-empty-PATH-elements.patch +Patch40: backport-CVE-2022-41953-is_Cygwin-avoid-exec-ing-anything.patch +Patch41: backport-CVE-2022-41953-Move-is_-platform-functions-to-the-beginning.patch +Patch42: backport-CVE-2022-41953-Move-the-_which-function-almost-to-the-top.patch +Patch43: backport-CVE-2022-41953-Work-around-Tcl-s-default-PATH-lookup.patch BuildRequires: gcc gettext BuildRequires: openssl-devel libcurl-devel expat-devel systemd asciidoc xmlto glib2-devel libsecret-devel pcre-devel desktop-file-utils @@ -299,6 +304,12 @@ make %{?_smp_mflags} test %{_mandir}/man7/git*.7.* %changelog +* Sat Jan 28 2023 fuanan - 2.27.0-11 +- Type:CVE +- ID:CVE-2022-41953 +- SUG:NA +- DESC:Fix CVE-2022-41953 + * Thu Jan 19 2023 fuanan - 2.27.0-10 - Type:CVE - ID:CVE-2022-23521 CVE-2022-41903 -- Gitee