Skip to content

extract_tar_archive: per-entry unpack loop lost directory deferral and dst canonicalization #508

Description

@thcp

Follow-up to #505 / #506. Found while auditing my own merged change.

extract_tar_archive was changed from Archive::unpack(dst) to a per-entry unpack_in loop in order to skip AppleDouble ._ members. That rewrite silently dropped two properties Archive::unpack provided.

What was lost

Archive::_unpack (tar-0.4.45 archive.rs:245-265) does this:

// Delay any directory entries until the end (they will be created if needed by
// descendants), to ensure that directory permissions do not interfere with
// descendant extraction.
let mut directories = Vec::new();
for entry in self._entries(None)? {
    let mut file = entry?;
    if file.header().entry_type() == crate::EntryType::Directory {
        directories.push(file);
    } else {
        file.unpack_in(dst)?;
    }
}
directories.sort_by(|a, b| b.path_bytes().cmp(&a.path_bytes()));
for mut dir in directories { dir.unpack_in(dst)?; }
  1. Directory deferral. Directories are buffered and applied last, reverse-sorted by path (tar-rs#242). Our loop creates them inline in archive order, so a directory member carrying a restrictive mode (e.g. 0o555, which ditto/tar record faithfully) is created before its children are written. The next file inside it then fails File::create with EACCES and unpack_without_apple_double returns Err, so first-run setup fails hard with no fallback.

  2. dst.canonicalize() up front, which on Windows supplies the \\?\ extended-length prefix so member paths over 260 characters still extract.

What was NOT lost

Traversal protection is intact. EntryFields::unpack_in still rejects ParentDir components, strips RootDir/Prefix, and calls validate_inside_dst, which canonicalizes on every entry. Zip-slip, absolute members and symlink escapes remain blocked. This is a robustness regression, not a security one.

Impact

Latent today. The published 0.16.0 pack contains no restrictive directory members: extracting it end to end produced 27,797 files and 0 sidecars. Windows takes the zip path for updates, so the long-path loss is currently theoretical.

Fix

Mirror upstream: buffer EntryType::Directory entries, apply them after the file pass reverse-sorted by path, and canonicalize destination before the loop.

Test

Extract an archive containing a 0o555 directory with children and assert it succeeds. Keep extract_tar_archive_drops_apple_double_sidecars green.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions