Block insecure options and protocols by default #1521

stsewd · 2022-12-24T01:45:22Z

This got a little longer than expected 😮‍💨, there were other places where git accepted ext:: URLs, like git pull/push/fetch <URL> https://git-scm.com/docs/git-remote-ext#_examples

And 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 with allow_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

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

Byron

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.

git/cmd.py

test/test_repo.py

git/remote.py

stsewd · 2022-12-28T01:12:50Z

I have added/updated the tests.

Byron

Thanks so much, this is tremendous work and great value for GitPython and all of its users. If you would like more recognition for this, please feel free to add an entry to changes.rst and include your name if you want. The same goes for the authors file.

That said, and if you feel you have a little more time and patience, I generally thought that asserting that these pwn 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!

test/test_remote.py

test/test_repo.py

stsewd · 2022-12-29T01:58:59Z

Thanks so much, this is tremendous work and great value for GitPython and all of its users. If you would like more recognition for this, please feel free to add an entry to changes.rst and include your name if you want. The same goes for the authors file.

Thank you!

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.

I'll try to update the PR later today or tomorrow

Byron · 2022-12-29T07:09:16Z

🙏🎉

* 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.

All versions of package gitpython are vulnerable to Remote Code Execution (RCE) due to improper user input validation, which makes it possible to inject a maliciously crafted remote URL into the clone command. Exploiting this vulnerability is possible because the library makes external calls to git without sufficient sanitization of input arguments. CVE: CVE-2022-24439 Upstream-Status: Backport Reference: gitpython-developers/GitPython#1529 gitpython-developers/GitPython#1518 gitpython-developers/GitPython#1521 Signed-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 to inject a maliciously crafted remote URL into the clone command. Exploiting this vulnerability is possible because the library makes external calls to git without sufficient sanitization of input arguments. CVE: CVE-2022-24439 Upstream-Status: Backport Reference: gitpython-developers/GitPython#1529 gitpython-developers/GitPython#1518 gitpython-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: poky MR: 124663 Type: Integration Disposition: Merged from poky ChangeID: 0721360 Description: All versions of package gitpython are vulnerable to Remote Code Execution (RCE) due to improper user input validation, which makes it possible to inject a maliciously crafted remote URL into the clone command. Exploiting this vulnerability is possible because the library makes external calls to git without sufficient sanitization of input arguments. CVE: CVE-2022-24439 Upstream-Status: Backport Reference: gitpython-developers/GitPython#1529 gitpython-developers/GitPython#1518 gitpython-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 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.

s-t-e-v-e-n-k and others added 2 commits December 23, 2022 16:16

Block unsafe options and protocols by default

e6108c7

Byron requested changes Dec 24, 2022

View reviewed changes

git/cmd.py Outdated Show resolved Hide resolved

test/test_repo.py Outdated Show resolved Hide resolved

git/remote.py Outdated Show resolved Hide resolved

stsewd added 4 commits December 27, 2022 16:56

Updates from review

fd2c6da

Update/add tests for Repo.clone*

b92f01a

More tests

c8ae33b

Submodule tests

9dc4392

stsewd marked this pull request as ready for review December 28, 2022 01:12

Byron approved these changes Dec 28, 2022

View reviewed changes

test/test_remote.py Show resolved Hide resolved

test/test_repo.py Show resolved Hide resolved

stsewd mentioned this pull request Dec 28, 2022

CVE-2022-24439: <gitpython::clone> 'ext::sh -c touch% /tmp/pwned' for remote code execution #1515

Closed

Updates from review

f4f2658

Byron added this to the v3.1.30 - Bugfixes milestone Dec 29, 2022

Byron merged commit 678a8fe into gitpython-developers:main Dec 29, 2022

Byron mentioned this pull request Dec 29, 2022

Quote URLs in Repo.clone_from() #1516

Closed

stsewd deleted the block-insecure-options branch December 29, 2022 13:53

HebaruSan mentioned this pull request Jan 16, 2023

[Bug] ValueError: SHA could not be resolved, git returned: b'' KSP-CKAN/xKAN-meta_testing#89

Closed

Beuc mentioned this pull request Jul 10, 2023

Block insecure non-multi options in clone/clone_from #1609

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Block insecure options and protocols by default #1521

Block insecure options and protocols by default #1521

stsewd commented Dec 24, 2022

Byron left a comment

stsewd commented Dec 28, 2022

Byron left a comment

stsewd commented Dec 29, 2022

Byron commented Dec 29, 2022

Block insecure options and protocols by default #1521

Block insecure options and protocols by default #1521

Conversation

stsewd commented Dec 24, 2022

Byron left a comment

Choose a reason for hiding this comment

stsewd commented Dec 28, 2022

Byron left a comment

Choose a reason for hiding this comment

stsewd commented Dec 29, 2022

Byron commented Dec 29, 2022