Bug 1224343 (CVE-2024-4981)

Summary: VUL-0: CVE-2024-4981: pagure: _update_file_in_git() follows symbolic links in temporary clones
Product: [openSUSE] openSUSE Distribution Reporter: SMASH SMASH <smash_bz>
Component: OtherAssignee: Neal Gompa <ngompa13>
Status: NEW --- QA Contact: Security Team bot <security-team>
Severity: Normal    
Priority: P3 - Medium CC: gianluca.gabrielli
Version: Leap 15.6   
Target Milestone: ---   
Hardware: Other   
OS: Other   
URL: https://smash.suse.de/issue/405936/
Whiteboard:
Found By: Security Response Team Services Priority:
Business Priority: Blocker: ---
Marketing QA Status: --- IT Deployment: ---

Description SMASH SMASH 2024-05-16 07:46:24 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.

References:
http://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2024-4981
https://bugzilla.redhat.com/show_bug.cgi?id=2280723
Comment 1 Gianluca Gabrielli 2024-05-16 07:49:24 UTC
Pagure is present on the following codestreams:
 - openSUSE:Backports:SLE-15-SP4/pagure
 - openSUSE:Backports:SLE-15-SP5/pagure
 - openSUSE:Backports:SLE-15-SP6/pagure
 - openSUSE:Factory/pagure