fix(utils): apply the stdlib data filter on both tarball extraction paths

comfy_cli/utils.py

@@ -169,31 +169,47 @@ def extract_tarball(
     shutil.rmtree(extractPath, ignore_errors=True)
     shutil.rmtree(outPath, ignore_errors=True)
 
+    # Both extraction paths below use the stdlib "data" filter so a member with an
+    # absolute path or a `../` traversal is rejected instead of being written
+    # wherever it points (CVE-2007-4559).
+    #
+    # This used to be skipped because of https://github.com/python/cpython/issues/107845,
+    # where data_filter resolved symlink targets against the destination root rather
+    # than against the directory holding the link, and so falsely raised
+    # LinkOutsideDestinationError on valid archives. That was a false-rejection bug,
+    # never an escape, and it was fixed in 3.10.13 / 3.11.5 / 3.12.0rc2 (2023-08-24).
+    # The only affected releases in our range are 3.10.12 and 3.11.4 — and since
+    # `extractall(filter=...)` does not exist before 3.10.12 at all, that is the whole
+    # window. On those two, the worst case is a loud error rather than a silent escape.
     if not show_progress:
         with tarfile.open(inPath) as tar:
-            tar.extractall(filter=None)
+            tar.extractall(filter="data")
         shutil.move(extractPath, outPath)
         return
 
     fileSize = inPath.stat().st_size
 
-    _size = 0
-
     with _tarball_progress("extracting tarball...", fileSize) as (barProg, barTask, pathProg, pathTask):
 
-        def _filter(tinfo: tarfile.TarInfo, _path: PathLike):
-            nonlocal _size
-            pathProg.update(pathTask, description=tinfo.path)
-            barProg.advance(barTask, _size)
-            _size = tinfo.size
-
-            # TODO: ideally we'd use data_filter here, but it's busted: https://github.com/python/cpython/issues/107845
-            # return tarfile.data_filter(tinfo, _path)
-            return tinfo
+        def _reporting_members(tar: tarfile.TarFile):
+            """Yield every member, driving the progress bars as we go.
+
+            Progress reporting used to ride on the ``filter`` argument, which
+            meant the extraction filter had to be a custom callable that
+            returned members unmodified — silently disabling the CVE-2007-4559
+            checks. The two concerns are separable: ``members`` drives the UI
+            and ``filter`` stays the stdlib ``"data"`` filter.
+            """
+            size = 0
+            for tinfo in tar:
+                pathProg.update(pathTask, description=tinfo.path)
+                barProg.advance(barTask, size)
+                size = tinfo.size
+                yield tinfo
+            barProg.advance(barTask, size)
 
         with tarfile.open(inPath) as tar:
-            tar.extractall(filter=_filter)
-        barProg.advance(barTask, _size)
+            tar.extractall(members=_reporting_members(tar), filter="data")
         pathProg.update(pathTask, description="")
 
     shutil.move(extractPath, outPath)

tests/comfy_cli/test_utils.py

@@ -1,4 +1,5 @@
 import io
+import tarfile
 from unittest.mock import MagicMock, patch
 
 import pytest
@@ -82,3 +83,79 @@ def test_create_and_extract(self, tmp_path, monkeypatch):
 
         assert (dest / "hello.txt").read_text() == "hello world"
         assert (dest / "sub" / "nested.txt").read_text() == "nested content"
+
+
+def _write_member(tar: tarfile.TarFile, name: str, data: bytes) -> None:
+    tinfo = tarfile.TarInfo(name)
+    tinfo.size = len(data)
+    tar.addfile(tinfo, io.BytesIO(data))
+
+
+def _write_symlink(tar: tarfile.TarFile, name: str, target: str) -> None:
+    tinfo = tarfile.TarInfo(name)
+    tinfo.type = tarfile.SYMTYPE
+    tinfo.linkname = target
+    tar.addfile(tinfo)
+
+
+class TestExtractTarballFiltering:
+    """Regression tests for CVE-2007-4559 (see issue #725).
+
+    ``extract_tarball`` extracts into the current working directory, so a member
+    named ``../evil.txt`` lands one level above it unless the stdlib ``data``
+    filter rejects the archive.
+    """
+
+    @staticmethod
+    def _traversal_tarball(workdir):
+        tarball = workdir / "payload.tgz"
+        with tarfile.open(tarball, "w:gz") as tar:
+            _write_member(tar, "payload/keep.txt", b"benign")
+            _write_member(tar, "../evil.txt", b"pwned")
+        return tarball
+
+    @pytest.mark.parametrize("show_progress", [False, True])
+    def test_rejects_path_traversal_member(self, tmp_path, monkeypatch, show_progress):
+        """Both extraction paths must refuse to write outside the destination."""
+        workdir = tmp_path / "work"
+        workdir.mkdir()
+        monkeypatch.chdir(workdir)
+
+        tarball = self._traversal_tarball(workdir)
+        escaped = tmp_path / "evil.txt"
+
+        rejection = None
+        with patch("comfy_cli.utils.Live"):
+            try:
+                extract_tarball(tarball, workdir / "out", show_progress=show_progress)
+            except tarfile.FilterError as exc:
+                rejection = exc
+
+        assert not escaped.exists(), f"traversal member escaped the extraction directory: {escaped}"
+        assert rejection is not None, "the traversal member was extracted instead of being rejected"
+
+    @pytest.mark.parametrize("show_progress", [False, True])
+    def test_allows_internal_symlinks(self, tmp_path, monkeypatch, show_progress):
+        """Control: the filter must not reject the layout real payloads use.
+
+        python-build-standalone tarballs (the only thing ``StandalonePython``
+        extracts) are full of relative symlinks such as ``bin/python3 ->
+        python3.12``. Those stay inside the destination and must survive.
+        """
+        workdir = tmp_path / "work"
+        workdir.mkdir()
+        monkeypatch.chdir(workdir)
+
+        tarball = workdir / "python.tgz"
+        with tarfile.open(tarball, "w:gz") as tar:
+            _write_member(tar, "python/bin/python3.12", b"#!/bin/sh\n")
+            _write_symlink(tar, "python/bin/python3", "python3.12")
+            _write_symlink(tar, "python/bin/python", "python3")
+
+        dest = workdir / "out"
+        with patch("comfy_cli.utils.Live"):
+            extract_tarball(tarball, dest, show_progress=show_progress)
+
+        assert (dest / "bin" / "python3.12").read_bytes() == b"#!/bin/sh\n"
+        assert (dest / "bin" / "python3").is_symlink()
+        assert (dest / "bin" / "python").is_symlink()