[jira] [Updated] (COMPRESS-727) Provide a symlink-resistant (and concurrency-safe) archive Extractor

["Tomas Illuminati (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/COMPRESS-727?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Tomas Illuminati updated COMPRESS-727:
--------------------------------------
    Attachment: COMPRESS-727.patch

> Provide a symlink-resistant (and concurrency-safe) archive Extractor
> --------------------------------------------------------------------
>
>                 Key: COMPRESS-727
>                 URL: https://issues.apache.org/jira/browse/COMPRESS-727
>             Project: Commons Compress
>          Issue Type: Improvement
>            Reporter: Piotr Karwasz
>            Priority: Major
>         Attachments: COMPRESS-727.patch
>
>
> h2. Summary
> {{ArchiveEntry.resolveIn(Path)}} protects against classic Zip-slip by resolving the entry name against the target directory, normalizing it, and rejecting the result if it no longer {{startsWith}} the target. This check is purely *lexical*: it manipulates path strings only. It does not look at the file system, so it does not defend against *symlink* attacks, and the example {{Expander}} that drives extraction does not create symlinks at all (so it is both lossy for legitimate archives and silent about the risk).
> This issue proposes a first-class, security-first {{Extractor}} that safely materializes a whole archive into a directory, including symlinks, and resists both symlink-slip from the archive itself and concurrent symlink races from third parties. The API is centered on {{Extractor.newExtractor(Path)}}, which returns a {{SecureExtractor}} when the platform supports {{java.nio.file.SecureDirectoryStream}}, or a best-effort {{Extractor}} otherwise.
> h2. Background: what resolveIn does and does not cover
> {code:java}
> default Path resolveIn(final Path parentPath) throws IOException {
>     final Path outputFile = parentPath.resolve(getName()).normalize();
>     if (!outputFile.startsWith(parentPath)) {
>         throw new ArchiveException("Zip slip ...");
>     }
>     return outputFile;
> }
> {code}
> This stops {{../../etc/passwd}} style names. It does *not* stop:
> * A symlink *entry inside the archive* (for example {{link -> /etc}} or {{link -> ../..}}) followed by a later entry written *through* that link ({{link/evil}}). The lexical check on {{target/link/evil}} passes because the string starts with {{target}}, but {{link}} is a symlink and the write escapes. This is "symlink-slip", and it defeats the lexical guard even when the target directory is fully trusted.
> * A symlink (or directory-to-symlink swap) planted *concurrently* by another process between the {{resolveIn}} check and the actual {{Files.newOutputStream}} call. The check and the write are not atomic with respect to the intermediate path components, so this is a classic TOCTOU race.
> The metadata needed to handle links is already exposed:
> * {{TarArchiveEntry}}: {{isSymbolicLink()}}, {{isLink()}} (hard link), {{getLinkName()}}, plus {{isBlockDevice()}} / {{isCharacterDevice()}} / {{isFIFO()}} for special types.
> * {{ZipArchiveEntry}}: {{isUnixSymlink()}}, with {{ZipFile.getUnixSymlink(entry)}} returning the link target.
> There is currently no use of {{SecureDirectoryStream}}, {{LinkOption.NOFOLLOW_LINKS}}, or {{Files.createSymbolicLink}} anywhere in main code, so this is greenfield.
> h2. Threat model
> # *Zip-slip* (path traversal via {{..}} or absolute names). Already handled by {{resolveIn}}.
> # *Symlink-slip from the archive* (trusted target directory). The archive plants a symlink and then writes through it, or supplies a symlink whose target points outside the extraction root. Must be handled by an extractor that controls symlink creation and never follows a link when materializing a child path.
> # *Concurrent symlink race* (untrusted / shared target directory). A third party with write access to the target tree swaps a path component for a symlink during extraction. Defeating this requires file-descriptor-relative, one-component-at-a-time resolution with no-follow semantics, that is, {{SecureDirectoryStream}}.
> h2. Real-life examples of untrusted / shared target directories
> These are settings where threat 3 is realistic because another user or process can write into the extraction tree while extraction runs:
> * *Shared CI / build agents*: multiple jobs extract into a common workspace or temp directory on the same host; one job races another.
> * *Multi-user package-manager caches*: extracting into group- or world-writable shared caches (Maven {{~/.m2}}, Gradle, pip, npm) on a shared build box.
> * *Web upload processing*: an upload is extracted into a directory that other worker processes or requests (possibly other tenants) can also write.
> * *Container image / layer extraction*: a layer's symlink redirects a later layer's write. Container runtimes specifically defend this (for example Docker/containerd use chroot-style joins, and Go's {{filepath-securejoin}} exists for exactly this).
> * *Background daemons*: antivirus, indexers, or backup agents racing the extractor over the same tree.
> Prior art that hardened against the same classes of bug:
> * The original Zip-slip disclosure (2018) prompted the lexical fix that {{resolveIn}} implements.
> * Python's {{tarfile}} extraction filters (PEP 706, shipped in 3.12, hardening the long-standing CVE-2007-4559) added {{data}} / {{tar}} / {{fully_trusted}} filters that, among other things, reject symlinks and links escaping the destination.
> * GNU tar performs "secure" extraction by removing an existing symlink before writing through its name and by deferring symlink creation.
> * Container runtimes rely on {{filepath-securejoin}} (Go) to resolve each component safely against symlink races.
> h2. Proposed API
> A supported {{Extractor}} type (promoted out of {{archivers.examples}} into, for example, {{org.apache.commons.compress.archivers}}), with a capability-detecting factory:
> {code:java}
> // Picks the strongest implementation the platform/filesystem allows.
> public static Extractor newExtractor(Path targetDirectory) throws IOException;
> {code}
> * On platforms where {{Files.newDirectoryStream(targetDirectory) instanceof SecureDirectoryStream}} (Unix-like systems, see {{sun.nio.fs.UnixSecureDirectoryStream}}), {{newExtractor}} returns a *{{SecureExtractor}}* that is resistant to both symlink-slip and concurrent symlink races.
> * Otherwise (notably Windows, or filesystems whose provider does not return a secure stream) it returns a base {{Extractor}} that defends symlink-slip from the archive on a best-effort basis but documents that it cannot fully close the TOCTOU race.
> The {{Extractor}} mirrors the entry points of today's {{Expander}} ({{ArchiveInputStream}}, {{ZipFile}}, {{TarFile}}, {{SevenZFile}}) and adds configuration:
> {code:java}
> Extractor extractor = Extractor.newExtractor(targetDir)
>         .setSymlinkPolicy(SymlinkPolicy.ALLOW_WITHIN_ROOT) // default REJECT
>         .setSpecialFilePolicy(SpecialFilePolicy.SKIP)      // devices, FIFOs
>         .setOverwrite(false);
> extractor.extract(archiveInputStream);
> {code}
> h3. SecureExtractor design
> Walk each entry's parent path *component by component* starting from an open directory handle on the (validated) extraction root:
> * For each intermediate component, open the subdirectory via {{SecureDirectoryStream.newDirectoryStream(name, LinkOption.NOFOLLOW_LINKS)}}. If a component is a symlink, refuse (it cannot be a legitimate parent under no-follow).
> * Create files relative to the innermost directory handle with no-follow / create-new semantics ({{newByteChannel}} with {{NOFOLLOW_LINKS}} + {{CREATE_NEW}}), so a concurrently planted symlink cannot redirect the write.
> * Create directories and symlinks relative to the directory handle, never via an absolute resolved path.
> Because every step is relative to a file descriptor rather than a re-resolved string path, a third party cannot win the race by swapping a parent component.
> h3. Symlink policy
> ||Policy||Behavior||
> |{{REJECT}} (default)|Fail if the archive contains any symlink entry.|
> |{{SKIP}}|Silently ignore symlink entries.|
> |{{ALLOW_WITHIN_ROOT}}|Create a symlink only if its target resolves to a location inside the extraction root; reject links escaping the root. Never follow the new link when writing later entries.|
> |{{ALLOW_ALL}}|Create symlinks verbatim (only for fully trusted archives).|
> h3. Entry-type handling
> ||Entry type||Default action||
> |Regular file|Extract (no-follow, create-new unless overwrite enabled).|
> |Directory|Create (no-follow).|
> |Symbolic link|Per {{SymlinkPolicy}} (default {{REJECT}}).|
> |Hard link (tar {{isLink()}})|Resolve link target within root, no-follow; reject if it escapes.|
> |Device / FIFO / other special|Per {{SpecialFilePolicy}} (default {{SKIP}}; never created by the secure path).|
> h2. Platform support and degradation
> * {{SecureDirectoryStream}} is available on Unix-like platforms via the default file system provider; {{newExtractor}} detects it with an {{instanceof}} check on a directory stream opened over the target.
> * On platforms without it, the base {{Extractor}} still validates each parent component with {{LinkOption.NOFOLLOW_LINKS}} (via {{Files.readAttributes}}) and refuses to traverse existing symlinks, which closes symlink-slip from the archive but remains non-atomic against a concurrent attacker. This residual limitation must be documented clearly.
> h2. Acceptance criteria
> * New {{Extractor}} (supported, not under {{examples}}) with {{newExtractor(Path)}} returning {{SecureExtractor}} when {{SecureDirectoryStream}} is available, else a best-effort {{Extractor}}.
> * Configurable {{SymlinkPolicy}} (default {{REJECT}}) and special-file handling; safe hard-link resolution.
> * {{SecureExtractor}} performs descriptor-relative, component-by-component, no-follow resolution and create-new writes.
> * Entry points covering {{ArchiveInputStream}}, {{ZipFile}}, {{TarFile}}, and {{SevenZFile}}, matching the existing {{Expander}} surface.
> * Tests: symlink-slip archive (link then write-through) is blocked under a trusted target; an escaping symlink target is rejected; a concurrent directory-to-symlink swap is defeated by {{SecureExtractor}} (and documented as not fully defeated by the base extractor); legitimate within-root symlinks extract correctly when allowed.
> * Javadoc plus a "safe extraction" section in the user guide, cross-referencing {{ArchiveEntry.resolveIn}} and the platform caveats.
> * Consider deprecating / redirecting {{Expander}} toward {{Extractor}}.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)