Skip to content

No defense against ../ relative path traversal during extraction #193

Description

@HaleTom

Summary

patool extracts archives into a temporary directory (_extract_archive) then optionally moves the result up one level (cleanup_outdir). This confines absolute-path attacks (backend tools strip leading /), but there is no central sanitization of ../ relative path components in archive member names. A crafted archive can escape the tempdir upward and write outside the intended extraction root.

This is a more detailed follow-up to the closed issue #155, which was closed as not_planned without discussion. That report was too terse to act on; this one supplies the concrete code anchors and a suggested fix.

Reproducer concept

A tar member named ../../../../etc/passwd extracted by tar --extract --file <archive> --directory <tempdir> (extract_tar) resolves relative to the tempdir CWD. The tempdir is created in . via fileutil.tmpdir(dir=".")tempfile.mkdtemp(prefix="Unpack_", dir="."), i.e. the user's CWD — so one ../ already reaches CWD, and each additional ../ climbs further up the tree. The file lands outside the intended extraction root.

Why the tempdir design isn't sufficient

  • The tempdir is created in . (__init__.py:393fileutil.py:77-79), i.e. the user's CWD — so one ../ already reaches CWD, and ../../.. reaches ancestors.
  • No extractor command uses tar's --no-unsafe-paths-style hardening; extract_tar passes only --extract --file --directory (tar.py:24-26). py_zipfile, 7z, unrar, unar, etc. pass no path-sanitizing flags either.
  • There is no central guard in _extract_archive: it creates outdir, builds the per-program cmdlist, runs it. Per-program modules do not filter member names for ...
  • cleanup_outdir / move_outdir_orphan move a single root entry up one level but never inspect for ...

Impact

A malicious archive extracted with patool can write files to arbitrary ancestor directories of the user's CWD (e.g. overwrite ~/.bashrc, drop a file in /etc if reachable from CWD). Severity depends on backend tool behavior and CWD depth, but there is no application-level guarantee that extraction stays within the root.

Suggested fix direction

A single guard in _extract_archive (or per-extractor) that rejects/sanitizes any member whose os.path.normpath resolves outside the extraction root — equivalent to tar's --no-unsafe-paths semantics — applied uniformly across all archive types rather than relying on each backend tool's flags.

Related

Question for maintainers

Would a PR implementing a central member-path sanitization guard be accepted? Is there a preferred approach (reject-and-error vs. strip-and-warn)?

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions