Bug 2280723 (CVE-2024-4981)
| Summary: | CVE-2024-4981 pagure: _update_file_in_git() follows symbolic links in temporary clones | ||
|---|---|---|---|
| Product: | [Other] Security Response | Reporter: | Nick Tait <ntait> |
| Component: | vulnerability | Assignee: | Product Security <prodsec-ir-bot> |
| Status: | CLOSED ERRATA | QA Contact: | |
| Severity: | high | Docs Contact: | |
| Priority: | high | ||
| Version: | unspecified | CC: | dominik, ngompa13, pingou, security-response-team |
| Target Milestone: | --- | Keywords: | Security |
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | --- | |
| Doc Text: |
A vulnerability was discovered in Pagure server. If a malicious user were to submit a git repository with symbolic links, the server could unintentionally show incorporate and make visible content from outside the git repo.
|
Story Points: | --- |
| Clone Of: | Environment: | ||
| Last Closed: | 2024-06-07 18:00:48 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
| Bug Depends On: | 2280724, 2280725 | ||
| Bug Blocks: | |||
Created pagure tracking bugs for this issue: Affects: epel-all [bug 2280724] Affects: fedora-all [bug 2280725] reported via https://bugzilla.redhat.com/show_bug.cgi?id=2278745 @ntait the vulnerability is fixed in pagure, new fedora packages are released as well. All related bugs are resolved, do you want to close this one too? Closing. |
Description of problem: In pagure/lib/git.py, the method _update_file_in_git() allows updating files on Pagure repositories directly from the web interface. Under the hood, it clones the repository to a temporary folder, performs the write operation, commits the changes and pushes it back to either the default branch or a new one. def _update_file_in_git( repo, branch, branchto, filename, content, message, user, email ): # [...] with TemporaryClone(repo, "main", "edit_file") as tempclone: # [...] file_path = os.path.join(newpath, filename) # [...] with open(file_path, "wb") as stream: stream.write(content.replace("\r", "").encode("utf-8")) # [...] new_repo.create_commit( # [...] ) # [...] tempclone.push( user.username, nbranch_ref.name if nbranch_ref else branch_ref.name, branchto, ) This code doesn't take enough precautions when dealing with symbolic links: if file_path points to one, open(file_path) will follow it. This link can point outside of the temporary clone folder. Version-Release number of selected component (if applicable): Likely introduced in commit 54335c2 in release 0.1.11, and verified on latest commit as of today (1b36cb8). How reproducible: This bug can be reliably exploited on the latest development version of Pagure; see steps below. Steps to Reproduce: 1. Create a new repository on a test Pagure instance; 2. Clone it locally; 3. From the local clone, run: ln -s /tmp/foo foo; 4. Commit this file, and push the commit back to Pagure; 5. From the web interface, in the "Files" tab, click on the one named foo, and then on the button "Edit"; 6. Put anything in the textarea, and then click on "Commit changes"; 7. Notice that the file /tmp/foo was created on the Pagure server. Actual results: Calls to _update_file_in_git() on symbolic links allow attackers to write fully controlled data to arbitrary paths (as long as the system user git has the right permissions on the destination). I could demonstrate the exploitation of this vulnerability and gain arbitrary code execution on stg.pagure.io by overriding /srv/git/.bashrc. As a proof, here's the output of name -a: Linux pagure-stg01.fedoraproject.org 4.18.0-513.11.1.el8_9.x86_64 #1 SMP Thu Dec 7 03:06:13 EST 2023 x86_64 x86_64 x86_64 GNU/Linux. I've since removed my changes to this file. Expected results: Calls to _update_file_in_git() on symbolic links should only be performed if the destination of link stays "within" the temporary clone folder. At first glance, I would not use os.readlink() here, as it would not catch cases where several links are chained; os.path.realpath() seems more appropriate. Additional info: I haven't had the time to work on a patch for this one, I'll try to submit it in the coming days.