python / python/cpython

Doc/library/tarfile.rst: document that the 'tar' extraction filter does not bound link targets

Open
#156,026 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

docs
Dominant language
Python
Stars
77.2k
Forks
35.9k
PR merge metrics
PR metrics pending

Description

Documentation

Documentation

Doc/library/tarfile.rst, the "Extraction filters" section — specifically tar_filter and the 'tar' entry in the filter list.

Summary

The 'tar' extraction filter does not check TarInfo.linkname. A symbolic link member whose target is an absolute path, or a relative path that climbs outside the destination, is extracted as given. Only 'data' refuses such members.

This is deliberate and consistent with PEP 706 — I am not proposing a behaviour change. The problem is that the documentation never states it, and one sentence in the tar_filter description reads as though it does the opposite.

Where the docs mislead

tar_filter's third bullet currently reads:

Refuse to extract files whose absolute path (after following symlinks) would end up outside the destination. This raises OutsideDestinationError.

The parenthetical "(after following symlinks)" describes os.path.realpath() applied to the member's own destination path — i.e. symlinks that already exist in the destination tree. It reads naturally as "symlink-related escapes are handled here".

That link targets are unchecked is currently derivable only by absence: the reader has to notice that data_filter is introduced with "In addition to what tar_filter does", and that link-target containment appears only in that second list. The non-guarantee is never stated positively, so nobody reading the tar_filter section can learn it from that section.

Why it matters in practice

I found this because five independent projects selected the 'tar' filter (or tarfile.tar_filter directly) in code whose stated purpose is safe extraction, leaving link targets unbounded as a result: unstructured-ingest, container-inspector, portage, poetry, and Samba's python/samba/safe_tarfile.py. Each has been reported to its maintainers separately; I am raising this here because five independent readings landing the same way looks like a docs problem rather than five coincidences.

Samba's case shows the mechanism most clearly, because the comment quotes the docs almost verbatim:

# The 'data' filter preserves no permissions so we select the
# intermediate 'tar' filter here which prevents escape but
# preserves permissions.
extraction_filter = staticmethod(tarfile.tar_filter)

"which prevents escape" is a restatement of the bullet above. That same class's pre-3.12 fallback implements _is_unsafe_symlink() and _is_unsafe_link() by hand — so the module became less strict when it moved to the stdlib filter, which is presumably the opposite of what was intended.

Measurement

Python 3.10.12 on Linux. The harness diffs the destination tree against a pre-extraction snapshot, so a member that was never created cannot be miscounted as written, and it runs with errorlevel=2 so nothing is silently skipped.

CONTROLS -- if these do not pass, nothing below means anything
  CONTROL-BENIGN plain file     filter=tar   -> extracted                          [ok]
  CONTROL-BENIGN plain file     filter=data  -> extracted                          [ok]
  CONTROL-DOTDOT member path    filter=tar   -> REFUSED (OutsideDestinationError)  [ok]
  CONTROL-DOTDOT member path    filter=data  -> REFUSED (OutsideDestinationError)  [ok]

PAYLOADS -- symlink TARGETS
  sym -> /etc                   filter=tar   -> LINK CREATED, POINTS OUTSIDE
  sym -> /etc                   filter=data  -> REFUSED (AbsoluteLinkError)
  sym -> /etc/passwd            filter=tar   -> LINK CREATED, POINTS OUTSIDE
  sym -> /etc/passwd            filter=data  -> REFUSED (AbsoluteLinkError)
  sym -> ../../OUTSIDE          filter=tar   -> LINK CREATED, POINTS OUTSIDE
  sym -> ../../OUTSIDE          filter=data  -> REFUSED (LinkOutsideDestinationError)

The CONTROL-DOTDOT row is the one that makes the rest meaningful: tar_filter genuinely does close the original CVE-2007-4559 path-traversal case, so "refused" is observable in this harness and the symlink rows are a real difference rather than a broken probe.

Reading through the resulting link works:

read through .../sandbox/lvl1/lvl2/dest/secrets
  -> /etc/passwd line 1: 'root:x:0:0:root:/root:/bin/bash'

Stated precisely so the proposed wording is not overclaimed: writing through such a link with a later member in the same archive is refused (OutsideDestinationError), because each member path is re-resolved at extraction time. Under 'tar' this is link placement and file read, not arbitrary write. Hard links to absolute outside paths failed here for an unrelated reason (ExtractError during link-target resolution), so I make no claim about them — the wording below covers symbolic links, which is what I measured.

Proposed change

Two edits, no behaviour change:

  1. In the filter list, the 'tar' entry gains one clause, since that list is where the choice between 'tar' and 'data' is actually made:
* the string ``'tar'``: Honor most *tar*-specific features (i.e. features of
  UNIX-like filesystems), but block features that are very likely to be
  surprising or malicious. This does not include limiting the targets of
  symbolic links. See :func:`tar_filter` for details.
  1. tar_filter gains a warning stating the non-guarantee positively:
  .. warning::

     This filter does not limit where a link member *points*.
     :attr:`TarInfo.linkname` is not checked, so a symbolic link whose target
     is an absolute path, or a relative path that climbs outside the
     destination, is extracted as given.
     The rule above constrains where each member is *written*; it does not
     constrain where a link resolves to.

     Only :func:`data_filter` refuses such members, raising
     :class:`~tarfile.AbsoluteLinkError` or
     :class:`~tarfile.LinkOutsideDestinationError`.

I have this as a patch and am happy to open the PR. I care that the fact appears somewhere in the tar_filter section — the phrasing is entirely yours to change.

Linked PRs
  • gh-156056

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Read Doc/library/tarfile.rst, focusing on the Extraction filters section, tar_filter, and the 'tar' filter entry. Review the proposed wording against PEP 706 and the documented data_filter behavior. Done means the documentation explicitly states that symbolic-link targets are not bounded by tar_filter, without changing behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
documentation
Issue type
Documentation
Difficulty
1/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.