Skip to content

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

Description

@drengir1

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    docsDocumentation in the Doc dir

    Projects

    Status
    Todo

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions