Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged

Conversation

stsewd
Copy link
Contributor

This got a little longer than expected 😮‍💨, there were other places where git acceptedext:: URLs, likegit 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 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

h3nnn4n and RemyDeWolf reacted with hooray emoji
s-t-e-v-e-n-kand others added2 commitsDecember 23, 2022 16:16
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
Copy link
Member

@ByronByron left a 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.

@stsewdstsewd marked this pull request as ready for reviewDecember 28, 2022 01:12
@stsewd
Copy link
ContributorAuthor

I have added/updated the tests.

Copy link
Member

@ByronByron left a 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!

anthonyjpratti and RemyDeWolf reacted with heart emoji
@stsewd
Copy link
ContributorAuthor

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

@ByronByron added this to thev3.1.30 - Bugfixes milestoneDec 29, 2022
@Byron
Copy link
Member

🙏🎉

@ByronByron merged commit678a8fe intogitpython-developers:mainDec 29, 2022
@stsewdstsewd deleted the block-insecure-options branchDecember 29, 2022 13:53
openstack-mirroring pushed a commit to openstack/openstack that referenced this pull requestJan 10, 2023
* 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
openstack-mirroring pushed a commit to openstack/requirements that referenced this pull requestJan 10, 2023
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull requestJan 20, 2023
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.
halstead pushed a commit to openembedded/openembedded-core that referenced this pull requestJan 26, 2023
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>
stefan-hartmann-lgs pushed a commit to hexagon-geo-surv/poky that referenced this pull requestJan 27, 2023
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>
jpuhlman pushed a commit to MontaVista-OpenSourceTechnology/poky that referenced this pull requestJan 31, 2023
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>
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull requestNov 16, 2023
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.)
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull requestNov 16, 2023
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.)
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull requestNov 16, 2023
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.)
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull requestNov 16, 2023
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.
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull requestNov 16, 2023
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.
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull requestNov 24, 2023
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.
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@ByronByronByron approved these changes

Assignees
No one assigned
Labels
None yet
Development

Successfully merging this pull request may close these issues.

CVE-2022-24439:<gitpython::clone> 'ext::sh -c touch% /tmp/pwned' for remote code execution
3 participants
@stsewd@Byron@s-t-e-v-e-n-k

[8]ページ先頭

©2009-2025 Movatter.jp