From c6ced5d61d1987da23d8e6b5aa8a03379f1f3691 Mon Sep 17 00:00:00 2001
From: Dominik Wombacher <dominik@wombacher.cc>
Date: May 24 2024 14:26:29 +0000
Subject: fix: generate_archive() follows symbolic links in temporary clones


If the repo contains symlinks, add actual link rather than the target content to the zip archive.
Avoids following symlinks and inclusion of data from outside the repository.

Vulnerability discovered by Thomas Chauchefoin <thomas@chauchefoin.fr>

Fixes: rhbz#2280030

Signed-off-by: Dominik Wombacher <dominik@wombacher.cc>

---

diff --git a/pagure/lib/git.py b/pagure/lib/git.py
index 025b83d..0224b0d 100644
--- a/pagure/lib/git.py
+++ b/pagure/lib/git.py
@@ -14,6 +14,7 @@ import json
 import logging
 import os
 import shutil
+import stat
 import subprocess
 import tarfile
 import tempfile
@@ -2842,7 +2843,17 @@ def generate_archive(project, commit, tag, name, archive_fmt):
             def addToZip(zf, path, zippath):
                 if _exclude_git(path):
                     return
-                if os.path.isfile(path):
+                # if path is symlink, add actual link and not target to zip
+                if os.path.islink(path):
+                    zi = zipfile.ZipInfo.from_file(path, zippath)
+                    zi.compress_type = zipfile.ZIP_DEFLATED
+                    # System which created zip, 3 = Unix; 0 = Windows
+                    zi.create_system = 3
+                    # mark as a symlink
+                    zi.external_attr |= stat.S_IFLNK << 16
+                    zf.writestr(zi, path)
+                    return
+                elif os.path.isfile(path):
                     zf.write(path, zippath, zipfile.ZIP_DEFLATED)
                 elif os.path.isdir(path):
                     if zippath:
diff --git a/tests/test_pagure_flask_ui_archives.py b/tests/test_pagure_flask_ui_archives.py
index aeac5f1..1da3d31 100644
--- a/tests/test_pagure_flask_ui_archives.py
+++ b/tests/test_pagure_flask_ui_archives.py
@@ -13,7 +13,7 @@ from __future__ import unicode_literals, absolute_import
 import unittest
 import sys
 import os
-import time
+from zipfile import ZipFile
 
 import mock
 import pygit2
@@ -123,6 +123,78 @@ class PagureFlaskUiArchivesTest(tests.Modeltests):
 
         self.assertEqual(os.listdir(self.archive_path), [])
 
+    # All tests against test3 repo complete, re-use repo for symlink test
+    def test_project_with_symlink_zip(self):
+        """Test that symlink rather than target file gets added to zip"""
+        filename = "os-release"
+        symlinkfile_target = "/etc/os-release"
+        directory = "etc"
+        symlinkdir_target = "/etc"
+        readme = "README.rst"
+        archivename = "test3.zip"
+        namespace = "somenamespace"
+        reponame = "test3"
+        repopath = os.path.join(
+            self.path, "repos", namespace, "%s.git" % reponame
+        )
+        repo = pygit2.Repository(repopath)
+        archivepath = os.path.join(self.archive_path, namespace, reponame)
+        tests.add_commit_git_repo(
+            repopath,
+            ncommits=1,
+            filename=filename,
+            symlink_to=symlinkfile_target,
+        )
+        tests.add_commit_git_repo(
+            repopath,
+            ncommits=1,
+            filename=directory,
+            symlink_to=symlinkdir_target,
+        )
+        tests.add_readme_git_repo(repopath)
+        commit = repo.head.target.hex
+
+        with mock.patch.dict(
+            "pagure.config.config",
+            {"ARCHIVE_FOLDER": os.path.join(self.path, "archives")},
+        ):
+            output = self.app.get(
+                "/%s/%s/archive/%s/%s"
+                % (namespace, reponame, commit, archivename),
+                follow_redirects=True,
+            )
+
+            self.assertEqual(output.status_code, 200)
+
+        # Was a subfolder for the correct commit created?
+        self.assertEqual(os.listdir(archivepath), [commit])
+
+        archivepath_commit = os.path.join(archivepath, commit)
+        # Does the subfolder contain the expected zip file?
+        self.assertEqual(os.listdir(archivepath_commit), [archivename])
+
+        archivepath_zip = os.path.join(archivepath_commit, archivename)
+        # Is the test file in the zip archive a symlink?
+        with ZipFile(os.path.join(archivepath_zip)) as testzip:
+            self.assertIn(
+                "lrw-r--r--",
+                str(testzip.getinfo("%s/%s" % (reponame, filename))),
+            )
+
+        # Is the test directory in the zip archive a symlink?
+        with ZipFile(os.path.join(archivepath_zip)) as testzip:
+            self.assertIn(
+                "?rwxr-xr-x",
+                str(testzip.getinfo("%s/%s/" % (reponame, directory))),
+            )
+
+        # Is the readme still an actual file in the zip archive?
+        with ZipFile(os.path.join(archivepath_zip)) as testzip:
+            self.assertIn(
+                "-rw-r--r--",
+                str(testzip.getinfo("%s/%s" % (reponame, readme))),
+            )
+
     def test_project_no_tag(self):
         """Test getting the archive of a non-empty project but without
         tags."""