-
-
Notifications
You must be signed in to change notification settings - Fork 908
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CVE-2022-24439: <gitpython::clone> 'ext::sh -c touch% /tmp/pwned'
for remote code execution
#1515
Comments
I am aware, have been informed and we can track it here. Thanks a lot for setting up this issue so timely. |
<gitpython::clone> 'ext::sh -c touch% /tmp/pwned'
for remote code execution
@Byron, had Snyk or Sam Wheating (@SamWheating?) contacted you previously about this or did you learn about it independently? I'm curious given there's no reference to any upstream report in the above Snyk report. |
I reached out to Snyk, who I believe got in touch with the maintainers. |
What makes you think that? Again, just curious given there doesn't seem to be any indication of that happening according to their report. Also, why did you go to Snyk rather than to upstream? |
Snyk did reach out to me by email, to my mind all this went pretty well. By publishing the issue the community can contribute a mitigation. |
Yeah, seems fine to me. It would have been nice to have an existing public report to go with the public release of the CVE so that all the people who handle CVEs (myself, and the reporter of this issue, for example) would know that the issue is already known to upstream and we don't have to spend time extracting that information via issues like this. Thank you for the insight. |
Looks like this same vulnerability has been reported in another git library (simple-git), https://www.cve.org/CVERecord?id=CVE-2022-25912. They added an option to opt-in in to the insecure behavior
And this should probably check for https://www.cve.org/CVERecord?id=CVE-2022-24433 too ( |
Snyk's vulnerability program is fantastic - you can report an issue to them and they will review it, triage it, try to get in contact with the maintainers and then register the CVE if applicable. It eliminates a lot of the overhead on my end and helps to ensure that a vulnerability is handled appropriately. I didn't have a direct line to the maintainers and I didn't want to open a public issue explaining a potentially sensitive vulnerability. In this case it sounds like Snyk was able to get in touch on my behalf and handle this disclosure responsibly. |
Just thought I would ping to keep this issue active. This is a critical issue in my org. Can we get a status update? Is a fix expected soon? |
|
Since the URL is passed directly to git clone, and the remote-ext helper will happily execute shell commands, so by default qoute URLs unless a (undocumented, on purpose) kwarg is passed. (CVE-2022-24439) Fixes gitpython-developers#1515
Since the URL is passed directly to git clone, and the remote-ext helper will happily execute shell commands, so by default disallow URLs that contain a "::" unless a new unsafe_protocols kwarg is passed. (CVE-2022-24439) Fixes gitpython-developers#1515
Since the URL is passed directly to git clone, and the remote-ext helper will happily execute shell commands, so by default disallow URLs that contain a "::" unless a new unsafe_protocols kwarg is passed. (CVE-2022-24439) Fixes gitpython-developers#1515
To avoid part of the problem, gitpython should also avoid interpreting positional arguments as option arguments. Line 1244 in 17ff263
but there are commands like git checkout that make special use of Other options could be:
|
Am I the only one confused by trying to fix a problem in the underlying Trying to do that, to my mind, will not deter an attacker who wants to find a bypass, and is much more likely to break downstream legitimate code, or use-cases for the now deemed unsafe features. There is clearly a tradeoff that seems impossible to get right.
Those who pass user-input to |
we're using this in eze https://github.com/RiverSafeUK/eze-cli
|
Thanks for sharing! Have you validated that URL injection works? After all the shell seems to be disabled by default and isn't enabled when calling clone either. Without shell in the middle a url like this should be just a single string passed to
For a moment I thought maybe |
Don't you agree that methods like clone that are expecting a path/url Line 1219 in 17ff263
--foo and that's interpreted as an option, that is unexpected/wrong.
|
I agree, and now I understand what the prior comment is about as well. And it seems not too unlikely that I think once it can be demonstrated that one can indeed execute programs on the server by manipulating the URL or maybe even the URL + destination path, that would be an issue specific to |
Since the URL is passed directly to git clone, and the remote-ext helper will happily execute shell commands, so by default disallow URLs that contain a "::" unless a new unsafe_protocols kwarg is passed. (CVE-2022-24439) Fixes gitpython-developers#1515
The library for |
Well, it does (https://libgit2.org/), doesn’t it? |
I assume @blade2005 meant that there is no native |
@Byron Could you please confirm if this PR #1518 is fixing CVE-2022-24439 issue as it has been added to 3.1.30 ? |
@nrpt-m I second that. |
A new release was created to incorporate many security related fixes. A special thanks goes to @stsewd who was a driving force behind implementing the fixes, and to the fine folks who discovered it. I hope this makes the upcoming year 2023 a little more secure for everyone 🎉. |
Can a release be made in Github? I imagine a nonzero number of people are watching for releases in this repository who aren't subscribed to this issue to be aware of the security impact of this release. |
Thanks for the hint. That should be done now. |
* Update requirements from branch 'master' to 2aaf64dd91c63aa55f4cbe8c037a6f545e91b302 - Merge "Bump GitPython to 3.1.30" - Bump GitPython to 3.1.30 3.1.30 includes 2 sets of fixes for CVE-2022-24439: https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-24439 gitpython-developers/GitPython#1515 PRs: gitpython-developers/GitPython#1518 gitpython-developers/GitPython#1521 Signed-off-by: Lon Hohberger <lhh@redhat.com> Change-Id: I0def2d9801f0b20fcc9b613165a29dbced1fd2d7
3.1.30 includes 2 sets of fixes for CVE-2022-24439: https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-24439 gitpython-developers/GitPython#1515 PRs: gitpython-developers/GitPython#1518 gitpython-developers/GitPython#1521 Signed-off-by: Lon Hohberger <lhh@redhat.com> Change-Id: I0def2d9801f0b20fcc9b613165a29dbced1fd2d7
3.1.30 - Make injections of command-invocations harder or impossible for clone and others. See gitpython-developers/GitPython#1518 for details. Note that this might constitute a breaking change for some users, and if so please let us know and we add an opt-out to this. - Prohibit insecure options and protocols by default, which is potentially a breaking change, but a necessary fix for gitpython-developers/GitPython#1515. Please take a look at the PR for more information and how to bypass these protections in case they cause breakage: gitpython-developers/GitPython#1521.
The tests of unsafe options are among those introduced originally in gitpython-developers#1521. They are regression tests for gitpython-developers#1515 (CVE-2022-24439). The unsafe options tests are paired: a test for the usual, default behavior of forbidding the option, and a test for the behavior when the option is explicitly allowed. Both tests use a payload that is intended to produce the side effect of a file of a specific name being created in a temporary directory. All the tests work on Unix-like systems. On Windows, the tests of the *allowed* cases are broken, and this commit marks them xfail. However, this has implications for the tests of the default, secure behavior, because until the "allowed" versions work on Windows, it will be unclear if either are using a payload that is effective and that corresponds to the way its effect is examined. (Fortunately, all are working on other OSes, and the affected code under test does not appear highly dependent on OS, so the fix is *probably* fully working on Windows as well.)
The tests of unsafe options are among those introduced originally in gitpython-developers#1521. They are regression tests for gitpython-developers#1515 (CVE-2022-24439). The unsafe options tests are paired: a test for the usual, default behavior of forbidding the option, and a test for the behavior when the option is explicitly allowed. Both tests use a payload that is intended to produce the side effect of a file of a specific name being created in a temporary directory. All the tests work on Unix-like systems. On Windows, the tests of the *allowed* cases are broken, and this commit marks them xfail. However, this has implications for the tests of the default, secure behavior, because until the "allowed" versions work on Windows, it will be unclear if either are using a payload that is effective and that corresponds to the way its effect is examined. (Fortunately, all are working on other OSes, and the affected code under test does not appear highly dependent on OS, so the fix is *probably* fully working on Windows as well.)
The tests of unsafe options are among those introduced originally in gitpython-developers#1521. They are regression tests for gitpython-developers#1515 (CVE-2022-24439). The unsafe options tests are paired: a test for the usual, default behavior of forbidding the option, and a test for the behavior when the option is explicitly allowed. In each such pair, both tests use a payload that is intended to produce the side effect of a file of a specific name being created in a temporary directory. All the tests work on Unix-like systems. On Windows, the tests of the *allowed* cases are broken, and this commit marks them xfail. However, this has implications for the tests of the default, secure behavior, because until the "allowed" versions work on Windows, it will be unclear if either are using a payload that is effective and that corresponds to the way its effect is examined. (Fortunately, all are working on other OSes, and the affected code under test does not appear highly dependent on OS, so the fix is *probably* fully working on Windows as well.)
The tests of unsafe options are among those introduced originally in gitpython-developers#1521. They are regression tests for gitpython-developers#1515 (CVE-2022-24439). The unsafe options tests are paired: a test for the usual, default behavior of forbidding the option, and a test for the behavior when the option is explicitly allowed. In each such pair, both tests use a payload that is intended to produce the side effect of a file of a specific name being created in a temporary directory. All the tests work on Unix-like systems. On Windows, the tests of the *allowed* cases are broken, and this commit marks them xfail. However, this has implications for the tests of the default, secure behavior, because until the "allowed" versions work on Windows, it will be unclear if either are using a payload that is effective and that corresponds to the way its effect is examined. Specifically, the "\" characters in the path seem to be treated as escape characters rather than literally. Also, "touch" is not a native Windows command, and the "touch" command in Git for Windows maps disallowed occurrences of ":" in filenames to a separate code point in the Private Use Area of the Basic Multilingual Plane.
The tests of unsafe options are among those introduced originally in gitpython-developers#1521. They are regression tests for gitpython-developers#1515 (CVE-2022-24439). The unsafe options tests are paired: a test for the usual, default behavior of forbidding the option, and a test for the behavior when the option is explicitly allowed. In each such pair, both tests use a payload that is intended to produce the side effect of a file of a specific name being created in a temporary directory. All the tests work on Unix-like systems. On Windows, the tests of the *allowed* cases are broken, and this commit marks them xfail. However, this has implications for the tests of the default, secure behavior, because until the "allowed" versions work on Windows, it will be unclear if either are using a payload that is effective and that corresponds to the way its effect is examined. What *seems* to happen is this: The "\" characters in the path are treated as shell escape characters rather than literally, with the effect of disappearing in most paths since most letters lack special meaning when escaped. Also, "touch" is not a native Windows command, and the "touch" command provided by Git for Windows is linked against MSYS2 libraries, causing it to map (some?) occurrences of ":" in filenames to a separate code point in the Private Use Area of the Basic Multilingual Plane. The result is a path with no directory separators or drive letter. It denotes a file of an unintended name in the current directory, which is never the intended location. The current directory depends on GitPython implementation details, but at present it's the top-level directory of the rw_repo working tree. A new unstaged file, named like "C\357\200\272UsersekAppDataLocalTemptmpc7x4xik5pwn", can be observed there (this is how "git status" will format the name). Fortunately, this and all related tests are working on other OSes, and the affected code under test does not appear highly dependent on OS. So the fix is *probably* fully working on Windows as well.
The tests of unsafe options are among those introduced originally in gitpython-developers#1521. They are regression tests for gitpython-developers#1515 (CVE-2022-24439). The unsafe options tests are paired: a test for the usual, default behavior of forbidding the option, and a test for the behavior when the option is explicitly allowed. In each such pair, both tests use a payload that is intended to produce the side effect of a file of a specific name being created in a temporary directory. All the tests work on Unix-like systems. On Windows, the tests of the *allowed* cases are broken, and this commit marks them xfail. However, this has implications for the tests of the default, secure behavior, because until the "allowed" versions work on Windows, it will be unclear if either are using a payload that is effective and that corresponds to the way its effect is examined. What *seems* to happen is this: The "\" characters in the path are treated as shell escape characters rather than literally, with the effect of disappearing in most paths since most letters lack special meaning when escaped. Also, "touch" is not a native Windows command, and the "touch" command provided by Git for Windows is linked against MSYS2 libraries, causing it to map (some?) occurrences of ":" in filenames to a separate code point in the Private Use Area of the Basic Multilingual Plane. The result is a path with no directory separators or drive letter. It denotes a file of an unintended name in the current directory, which is never the intended location. The current directory depends on GitPython implementation details, but at present it's the top-level directory of the rw_repo working tree. A new unstaged file, named like "C\357\200\272UsersekAppDataLocalTemptmpc7x4xik5pwn", can be observed there (this is how "git status" will format the name). Fortunately, this and all related tests are working on other OSes, and the affected code under test does not appear highly dependent on OS. So the fix is *probably* fully working on Windows as well.
This appeared in the CVE feed today: https://security.snyk.io/vuln/SNYK-PYTHON-GITPYTHON-3113858
Not sure if this was reported to you before or not, reporting it here just in case.
The text was updated successfully, but these errors were encountered: