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: vulnerabilityAssignee: Product Security <prodsec-ir-bot>
Status: CLOSED ERRATA QA Contact:
Severity: high Docs Contact:
Priority: high    
Version: unspecifiedCC: 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:    

Description Nick Tait 2024-05-15 22:51:04 UTC
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.

Comment 1 Nick Tait 2024-05-15 22:51:22 UTC
Created pagure tracking bugs for this issue:

Affects: epel-all [bug 2280724]
Affects: fedora-all [bug 2280725]

Comment 3 Nick Tait 2024-05-15 22:56:24 UTC
reported via https://bugzilla.redhat.com/show_bug.cgi?id=2278745

Comment 4 Dominik Wombacher 2024-06-06 09:02:18 UTC
@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?

Comment 5 Nick Tait 2024-06-07 18:00:48 UTC
Closing.