Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork937
Block insecure options and protocols by default#1521
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Since the URL is passed directly to git clone, and the remote-ext helperwill happily execute shell commands, so by default disallow URLs thatcontain a "::" unless a new unsafe_protocols kwarg is passed.(CVE-2022-24439)Fixesgitpython-developers#1515
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks so much for having put in so much time and effort to help fixing this!
It mostly looks good to me, and once CI is working there shouldn't be much in the way of merging the PR.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
I have added/updated the tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks so much, this is tremendous work and great value forGitPython
and all of its users. If you would like more recognition for this, please feel free to add an entry tochanges.rst
and include your name if you want. The same goes for theauthors
file.
That said, and if you feel you have a little more time and patience, I generally thought that asserting that thesepwn
files exist or don't exists where it's easily possible would help readability a lot while adding some assurance that it's actually, effectively working like it should. After all, having an exception raised is a side-effect of us ideally stopping the git command to be executed, but we don't really know unless we fail to observe its side-effect that we are trying to prevent.
If you don't have time, that's alright as well, just let me know and I will merge as is and get a new release ready.
Thanks so much!
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Thank you!
I'll try to update the PR later today or tomorrow |
🙏🎉 |
* 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 forCVE-2022-24439:https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-24439gitpython-developers/GitPython#1515 PRs:gitpython-developers/GitPython#1518gitpython-developers/GitPython#1521 Signed-off-by: Lon Hohberger <lhh@redhat.com> Change-Id: I0def2d9801f0b20fcc9b613165a29dbced1fd2d7
3.1.30 includes 2 sets of fixes forCVE-2022-24439:https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-24439gitpython-developers/GitPython#1515PRs:gitpython-developers/GitPython#1518gitpython-developers/GitPython#1521Signed-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. Seegitpython-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 forgitpython-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.
All versions of package gitpython are vulnerable to Remote Code Execution(RCE) due to improper user input validation, which makes it possible toinject a maliciously crafted remote URL into the clone command. Exploitingthis vulnerability is possible because the library makes external calls togit without sufficient sanitization of input arguments.CVE:CVE-2022-24439Upstream-Status: BackportReference:gitpython-developers/GitPython#1529gitpython-developers/GitPython#1518gitpython-developers/GitPython#1521Signed-off-by: Narpat Mali <narpat.mali@windriver.com>
All versions of package gitpython are vulnerable to Remote Code Execution(RCE) due to improper user input validation, which makes it possible toinject a maliciously crafted remote URL into the clone command. Exploitingthis vulnerability is possible because the library makes external calls togit without sufficient sanitization of input arguments.CVE:CVE-2022-24439Upstream-Status: BackportReference:gitpython-developers/GitPython#1529gitpython-developers/GitPython#1518gitpython-developers/GitPython#1521(From OE-Core rev: 55f93e3786290dfa5ac72b5969bb2793f6a98bde)Signed-off-by: Narpat Mali <narpat.mali@windriver.com>Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
Source: pokyMR: 124663Type: IntegrationDisposition: Merged from pokyChangeID:0721360Description:All versions of package gitpython are vulnerable to Remote Code Execution(RCE) due to improper user input validation, which makes it possible toinject a maliciously crafted remote URL into the clone command. Exploitingthis vulnerability is possible because the library makes external calls togit without sufficient sanitization of input arguments.CVE:CVE-2022-24439Upstream-Status: BackportReference:gitpython-developers/GitPython#1529gitpython-developers/GitPython#1518gitpython-developers/GitPython#1521(From OE-Core rev: 55f93e3786290dfa5ac72b5969bb2793f6a98bde)Signed-off-by: Narpat Mali <narpat.mali@windriver.com>Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>Signed-off-by: Jeremy A. Puhlman <jpuhlman@mvista.com>
The tests of unsafe options are among those introduced originallyingitpython-developers#1521. They are regression tests forgitpython-developers#1515 (CVE-2022-24439).The unsafe options tests are paired: a test for the usual, defaultbehavior of forbidding the option, and a test for the behavior whenthe option is explicitly allowed. Both tests use a payload that isintended to produce the side effect of a file of a specific namebeing created in a temporary directory.All the tests work on Unix-like systems. On Windows, the tests ofthe *allowed* cases are broken, and this commit marks them xfail.However, this has implications for the tests of the default, securebehavior, because until the "allowed" versions work on Windows, itwill be unclear if either are using a payload that is effective andthat corresponds to the way its effect is examined. (Fortunately,all are working on other OSes, and the affected code under testdoes 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 originallyingitpython-developers#1521. They are regression tests forgitpython-developers#1515 (CVE-2022-24439).The unsafe options tests are paired: a test for the usual, defaultbehavior of forbidding the option, and a test for the behavior whenthe option is explicitly allowed. Both tests use a payload that isintended to produce the side effect of a file of a specific namebeing created in a temporary directory.All the tests work on Unix-like systems. On Windows, the tests ofthe *allowed* cases are broken, and this commit marks them xfail.However, this has implications for the tests of the default, securebehavior, because until the "allowed" versions work on Windows, itwill be unclear if either are using a payload that is effective andthat corresponds to the way its effect is examined. (Fortunately,all are working on other OSes, and the affected code under testdoes 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 originallyingitpython-developers#1521. They are regression tests forgitpython-developers#1515 (CVE-2022-24439).The unsafe options tests are paired: a test for the usual, defaultbehavior of forbidding the option, and a test for the behavior whenthe option is explicitly allowed. In each such pair, both tests usea payload that is intended to produce the side effect of a file ofa specific name being created in a temporary directory.All the tests work on Unix-like systems. On Windows, the tests ofthe *allowed* cases are broken, and this commit marks them xfail.However, this has implications for the tests of the default, securebehavior, because until the "allowed" versions work on Windows, itwill be unclear if either are using a payload that is effective andthat corresponds to the way its effect is examined. (Fortunately,all are working on other OSes, and the affected code under testdoes 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 originallyingitpython-developers#1521. They are regression tests forgitpython-developers#1515 (CVE-2022-24439).The unsafe options tests are paired: a test for the usual, defaultbehavior of forbidding the option, and a test for the behavior whenthe option is explicitly allowed. In each such pair, both tests usea payload that is intended to produce the side effect of a file ofa specific name being created in a temporary directory.All the tests work on Unix-like systems. On Windows, the tests ofthe *allowed* cases are broken, and this commit marks them xfail.However, this has implications for the tests of the default, securebehavior, because until the "allowed" versions work on Windows, itwill be unclear if either are using a payload that is effective andthat corresponds to the way its effect is examined.Specifically, the "\" characters in the path seem to be treated asescape characters rather than literally. Also, "touch" is not anative Windows command, and the "touch" command in Git for Windowsmaps disallowed occurrences of ":" in filenames to a separate codepoint in the Private Use Area of the Basic Multilingual Plane.
The tests of unsafe options are among those introduced originallyingitpython-developers#1521. They are regression tests forgitpython-developers#1515 (CVE-2022-24439).The unsafe options tests are paired: a test for the usual, defaultbehavior of forbidding the option, and a test for the behavior whenthe option is explicitly allowed. In each such pair, both tests usea payload that is intended to produce the side effect of a file ofa specific name being created in a temporary directory.All the tests work on Unix-like systems. On Windows, the tests ofthe *allowed* cases are broken, and this commit marks them xfail.However, this has implications for the tests of the default, securebehavior, because until the "allowed" versions work on Windows, itwill be unclear if either are using a payload that is effective andthat corresponds to the way its effect is examined.What *seems* to happen is this: The "\" characters in the path aretreated as shell escape characters rather than literally, with theeffect of disappearing in most paths since most letters lackspecial meaning when escaped. Also, "touch" is not a native Windowscommand, and the "touch" command provided by Git for Windows islinked against MSYS2 libraries, causing it to map (some?)occurrences of ":" in filenames to a separate code point in thePrivate Use Area of the Basic Multilingual Plane. The result is apath with no directory separators or drive letter. It denotes afile of an unintended name in the current directory, which is neverthe intended location. The current directory depends on GitPythonimplementation details, but at present it's the top-level directoryof the rw_repo working tree. A new unstaged file, named like"C\357\200\272UsersekAppDataLocalTemptmpc7x4xik5pwn", can beobserved 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 dependenton OS. So the fix is *probably* fully working on Windows as well.
The tests of unsafe options are among those introduced originallyingitpython-developers#1521. They are regression tests forgitpython-developers#1515 (CVE-2022-24439).The unsafe options tests are paired: a test for the usual, defaultbehavior of forbidding the option, and a test for the behavior whenthe option is explicitly allowed. In each such pair, both tests usea payload that is intended to produce the side effect of a file ofa specific name being created in a temporary directory.All the tests work on Unix-like systems. On Windows, the tests ofthe *allowed* cases are broken, and this commit marks them xfail.However, this has implications for the tests of the default, securebehavior, because until the "allowed" versions work on Windows, itwill be unclear if either are using a payload that is effective andthat corresponds to the way its effect is examined.What *seems* to happen is this: The "\" characters in the path aretreated as shell escape characters rather than literally, with theeffect of disappearing in most paths since most letters lackspecial meaning when escaped. Also, "touch" is not a native Windowscommand, and the "touch" command provided by Git for Windows islinked against MSYS2 libraries, causing it to map (some?)occurrences of ":" in filenames to a separate code point in thePrivate Use Area of the Basic Multilingual Plane. The result is apath with no directory separators or drive letter. It denotes afile of an unintended name in the current directory, which is neverthe intended location. The current directory depends on GitPythonimplementation details, but at present it's the top-level directoryof the rw_repo working tree. A new unstaged file, named like"C\357\200\272UsersekAppDataLocalTemptmpc7x4xik5pwn", can beobserved 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 dependenton OS. So the fix is *probably* fully working on Windows as well.
This got a little longer than expected 😮💨, there were other places where git accepted
ext::
URLs, likegit pull/push/fetch <URL>
https://git-scm.com/docs/git-remote-ext#_examplesAnd there are other config options that can be harmful, so I think we should just forbid the
--config
option, if anyone is relying on that option, they can opt-out withallow_unsafe_options=True
.--*-pack
and--exec
are the options that I found that could lead to RCE, but anyone allowing users to pass arbitrary options should be aware that it may be more of these, don't know.This is still missing adding/updating tests.
This is on top of#1516
Fixes#1515