diff --git a/src/changes/changes.xml b/src/changes/changes.xml index cb066c516c2..687c00b8409 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -209,6 +209,7 @@ The type attribute can be add,update,fix,remove. ArchiveStreamFactory now throws ArchiveException instead of IllegalArgumetException/IllegalStateException. ExactMath now throws ArchiveException instead of IllegalArgumetException/IllegalStateException. + Add a security-first Extractor (org.apache.commons.compress.archivers.extractor) that safely extracts archives into a directory, including symbolic links, resisting symlink-slip and, on platforms with java.nio.file.SecureDirectoryStream, concurrent symlink races. Add MemoryLimitException.MemoryLimitException(long, long). Add CompressException.CompressException(String, Object...). Add ArchiveException.ArchiveException(String, Object...). diff --git a/src/main/java/org/apache/commons/compress/archivers/extractor/Extractor.java b/src/main/java/org/apache/commons/compress/archivers/extractor/Extractor.java new file mode 100644 index 00000000000..49da58b0439 --- /dev/null +++ b/src/main/java/org/apache/commons/compress/archivers/extractor/Extractor.java @@ -0,0 +1,427 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.commons.compress.archivers.extractor; + +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.nio.channels.Channels; +import java.nio.file.DirectoryStream; +import java.nio.file.Files; +import java.nio.file.InvalidPathException; +import java.nio.file.LinkOption; +import java.nio.file.NoSuchFileException; +import java.nio.file.Path; +import java.nio.file.SecureDirectoryStream; +import java.nio.file.StandardOpenOption; +import java.nio.file.attribute.BasicFileAttributes; +import java.util.Enumeration; +import java.util.Objects; + +import org.apache.commons.compress.archivers.ArchiveEntry; +import org.apache.commons.compress.archivers.ArchiveException; +import org.apache.commons.compress.archivers.ArchiveInputStream; +import org.apache.commons.compress.archivers.tar.TarArchiveEntry; +import org.apache.commons.compress.archivers.tar.TarFile; +import org.apache.commons.compress.archivers.zip.ZipArchiveEntry; +import org.apache.commons.compress.archivers.zip.ZipFile; +import org.apache.commons.io.IOUtils; + +/** + * Safely materializes an archive into a target directory, including symbolic links, while resisting both symlink-slip from + * the archive itself and concurrent symlink races from third parties. + *

+ * Obtain an instance with {@link #newExtractor(Path)}, which returns the strongest implementation the platform supports: a + * race-safe extractor backed by {@link SecureDirectoryStream} where the file system provides one (Linux), otherwise this + * best-effort base implementation. The base implementation resolves every path component with + * {@link LinkOption#NOFOLLOW_LINKS} and writes regular files {@code CREATE_NEW}, which stops symlink-slip from the archive and + * silent overwrites, but it cannot fully close a concurrent time-of-check-to-time-of-use race; that residual is documented + * and only the secure implementation removes it. + *

+ *

+ * Defaults are conservative: symbolic-link entries are rejected ({@link SymlinkPolicy#REJECT}), special entries are skipped + * ({@link SpecialFilePolicy#SKIP}), and existing files are not overwritten. Instances are not thread-safe: configure an + * instance before extracting and do not share it across concurrent extractions. + *

+ * + * @since 1.29.0 + */ +public class Extractor { + + enum EntryType { + FILE, DIRECTORY, SYMLINK, HARD_LINK, SPECIAL + } + + /** + * Creates an extractor for {@code targetDirectory}, returning a race-safe implementation when the platform's file system + * provider exposes a {@link SecureDirectoryStream} (Linux), otherwise a best-effort implementation. + * + * @param targetDirectory an existing directory into which archives are extracted; resolved to its real path. + * @return an extractor bound to {@code targetDirectory}. + * @throws IOException if {@code targetDirectory} does not exist or is not a directory. + * @since 1.29.0 + */ + public static Extractor newExtractor(final Path targetDirectory) throws IOException { + final Path root = Objects.requireNonNull(targetDirectory, "targetDirectory").toRealPath(); + if (!Files.isDirectory(root)) { + throw new IOException("Target is not a directory: " + targetDirectory); + } + try (DirectoryStream stream = Files.newDirectoryStream(root)) { + if (stream instanceof SecureDirectoryStream) { + return new SecureExtractor(root); + } + } + return new Extractor(root); + } + + /** The validated, canonical extraction root. */ + final Path rootDirectory; + + private SymlinkPolicy symlinkPolicy = SymlinkPolicy.REJECT; + + private SpecialFilePolicy specialFilePolicy = SpecialFilePolicy.SKIP; + + private boolean overwrite; + + private Runnable beforeLeafWrite; + + Extractor(final Path rootDirectory) { + this.rootDirectory = rootDirectory; + } + + private static EntryType classify(final ArchiveEntry entry) { + if (entry instanceof TarArchiveEntry) { + final TarArchiveEntry tar = (TarArchiveEntry) entry; + if (tar.isSymbolicLink()) { + return EntryType.SYMLINK; + } + if (tar.isLink()) { + return EntryType.HARD_LINK; + } + if (tar.isBlockDevice() || tar.isCharacterDevice() || tar.isFIFO()) { + return EntryType.SPECIAL; + } + return tar.isDirectory() ? EntryType.DIRECTORY : EntryType.FILE; + } + if (entry instanceof ZipArchiveEntry && ((ZipArchiveEntry) entry).isUnixSymlink()) { + return EntryType.SYMLINK; + } + return entry.isDirectory() ? EntryType.DIRECTORY : EntryType.FILE; + } + + /** + * Extracts the entries of an {@link ArchiveInputStream}. + *

+ * Zip unix symbolic links are recognized only through {@link #extract(ZipFile)}: their unix mode lives in the central + * directory, which a stream does not expose, so a zip symbolic link presented through a stream is materialized as a + * regular file (no-follow, create-new) rather than a link. + *

+ * + * @param archive the archive to extract; not closed by this method. + * @throws IOException if extraction fails or a policy rejects an entry. + * @since 1.29.0 + */ + public void extract(final ArchiveInputStream archive) throws IOException { + ArchiveEntry entry; + while ((entry = archive.getNextEntry()) != null) { + if (!archive.canReadEntryData(entry)) { + continue; + } + final EntryType type = classify(entry); + String linkTarget = null; + if (type == EntryType.SYMLINK || type == EntryType.HARD_LINK) { + // Only tar carries link metadata through a stream: a zip unix symbolic link is recognized solely from the + // central directory (see extract(ZipFile)), which a stream does not expose, so classify never reports a + // streamed entry as a link unless it is a TarArchiveEntry. + linkTarget = ((TarArchiveEntry) entry).getLinkName(); + } + process(entry.getName(), type, linkTarget, type == EntryType.FILE ? archive : null); + } + } + + /** + * Extracts the entries of a {@link TarFile}. + * + * @param archive the archive to extract; not closed by this method. + * @throws IOException if extraction fails or a policy rejects an entry. + * @since 1.29.0 + */ + public void extract(final TarFile archive) throws IOException { + for (final TarArchiveEntry entry : archive.getEntries()) { + final EntryType type = classify(entry); + if (type == EntryType.FILE) { + try (InputStream content = archive.getInputStream(entry)) { + process(entry.getName(), type, null, content); + } + } else { + final String linkTarget = type == EntryType.SYMLINK || type == EntryType.HARD_LINK ? entry.getLinkName() : null; + process(entry.getName(), type, linkTarget, null); + } + } + } + + /** + * Extracts the entries of a {@link ZipFile}. + * + * @param archive the archive to extract; not closed by this method. + * @throws IOException if extraction fails or a policy rejects an entry. + * @since 1.29.0 + */ + public void extract(final ZipFile archive) throws IOException { + final Enumeration entries = archive.getEntries(); + while (entries.hasMoreElements()) { + final ZipArchiveEntry entry = entries.nextElement(); + if (!archive.canReadEntryData(entry)) { + continue; + } + final EntryType type = classify(entry); + if (type == EntryType.FILE) { + try (InputStream content = archive.getInputStream(entry)) { + process(entry.getName(), type, null, content); + } + } else { + final String linkTarget = type == EntryType.SYMLINK ? archive.getUnixSymlink(entry) : null; + process(entry.getName(), type, linkTarget, null); + } + } + } + + boolean isOverwrite() { + return overwrite; + } + + final void runBeforeLeafWrite() { + if (beforeLeafWrite != null) { + beforeLeafWrite.run(); + } + } + + final void setBeforeLeafWrite(final Runnable hook) { + this.beforeLeafWrite = hook; + } + + /** + * Tests whether {@code path} is contained within the canonical extraction root, comparing component by component so a + * sibling that merely shares a name prefix (for example {@code root-old} beside {@code root}) is not treated as contained. + * This is a string comparison; it never touches the file system. + */ + private boolean isWithinRoot(final Path path) { + return path.startsWith(rootDirectory); + } + + /** + * Resolves {@code name} against the extraction root and applies the lexical zip-slip guard. + *

+ * The guard is lexical by design and must stay free of file system access: {@code name} is untrusted until + * {@link #isWithinRoot(Path)} has accepted it, and on Windows a UNC name such as {@code \\host\share} would open a + * connection to {@code host} the moment a {@code Files} method or {@link Path#toRealPath(LinkOption...)} probed it. + * {@code resolve}, {@code normalize}, {@code equals} and {@code startsWith} inspect the path string only. + *

+ * + * @return the resolved path within the root, or {@code null} if {@code name} resolves to the root itself (for example + * {@code a/..}), which carries nothing to materialize; the caller skips such entries rather than writing at or + * replacing the root. + * @throws ArchiveException if the resolved path escapes the extraction root. + */ + private Path resolveWithinRoot(final String name) throws ArchiveException { + final Path resolved = rootDirectory.resolve(name).normalize(); + if (resolved.equals(rootDirectory)) { + return null; + } + // A component of only dots or spaces (for example "...") is stripped by Windows and can alias the root, which this + // lexical equality does not catch. The entry still stays within the root and its no-follow CREATE_NEW write fails + // closed rather than replacing the root, so this is a known corner case rather than a boundary escape. + if (!isWithinRoot(resolved)) { + throw new ArchiveException("Entry '%s' would escape the extraction root", name); + } + return resolved; + } + + /** + * Walks the path from the root to {@code directory}, creating missing components and refusing to traverse any existing + * component that is a symbolic link or a non-directory. This is the best-effort, non-atomic parent resolution; the secure + * implementation overrides the leaf operations to be descriptor-relative. + */ + void ensureDirectory(final Path directory) throws IOException { + if (directory == null || directory.equals(rootDirectory)) { + return; + } + Path current = rootDirectory; + for (final Path component : rootDirectory.relativize(directory)) { + current = current.resolve(component); + final BasicFileAttributes attrs; + try { + attrs = Files.readAttributes(current, BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS); + } catch (final IOException e) { + // readAttributes documents only IOException, and NoSuchFileException is an optional specific exception, so + // confirm the component is genuinely missing before creating it and rethrow any other I/O error. + if (e instanceof NoSuchFileException || Files.notExists(current, LinkOption.NOFOLLOW_LINKS)) { + Files.createDirectory(current); + continue; + } + throw e; + } + if (attrs.isSymbolicLink()) { + throw new ArchiveException("Refusing to traverse symbolic link: %s", current); + } + if (!attrs.isDirectory()) { + throw new ArchiveException("Parent path component is not a directory: %s", current); + } + } + } + + private void process(final String name, final EntryType type, final String linkTarget, final InputStream content) throws IOException { + try { + processEntry(name, type, linkTarget, content); + } catch (final InvalidPathException e) { + // A name or link target that cannot become a Path on this platform (for example one containing a NUL byte) is + // rejected as a malformed entry rather than propagated as an unchecked exception that would abort extraction. + throw new ArchiveException("Invalid entry name: " + name, e); + } + } + + private void processEntry(final String name, final EntryType type, final String linkTarget, final InputStream content) throws IOException { + if (type == EntryType.SPECIAL) { + if (specialFilePolicy == SpecialFilePolicy.REJECT) { + throw new ArchiveException("Special entry not allowed: %s", name); + } + return; + } + if (type == EntryType.SYMLINK) { + if (symlinkPolicy == SymlinkPolicy.REJECT) { + throw new ArchiveException("Symbolic link not allowed: %s", name); + } + if (symlinkPolicy == SymlinkPolicy.SKIP) { + return; + } + } + final Path leaf = resolveWithinRoot(name); + if (leaf == null) { + // The entry resolves to the extraction root itself; there is nothing to materialize and writing here would + // target or replace the root, so skip it. + return; + } + ensureDirectory(leaf.getParent()); + switch (type) { + case DIRECTORY: + ensureDirectory(leaf); + break; + case FILE: + writeFile(leaf, content); + break; + case SYMLINK: + writeSymbolicLink(leaf, linkTarget); + break; + case HARD_LINK: + writeHardLink(leaf, linkTarget); + break; + default: + throw new ArchiveException("Unsupported entry type for '%s'", name); + } + } + + /** + * Sets whether existing files may be overwritten. When {@code false} (the default) an entry whose target already exists + * fails the extraction; the write never follows an existing symbolic link at the target path. + * + * @param overwrite whether to overwrite existing files. + * @return {@code this}. + * @since 1.29.0 + */ + public Extractor setOverwrite(final boolean overwrite) { + this.overwrite = overwrite; + return this; + } + + /** + * Sets the policy for special entries (devices, FIFOs). Defaults to {@link SpecialFilePolicy#SKIP}. + * + * @param specialFilePolicy the policy to apply; not null. + * @return {@code this}. + * @since 1.29.0 + */ + public Extractor setSpecialFilePolicy(final SpecialFilePolicy specialFilePolicy) { + this.specialFilePolicy = Objects.requireNonNull(specialFilePolicy, "specialFilePolicy"); + return this; + } + + /** + * Sets the policy for symbolic-link entries. Defaults to {@link SymlinkPolicy#REJECT}. + * + * @param symlinkPolicy the policy to apply; not null. + * @return {@code this}. + * @since 1.29.0 + */ + public Extractor setSymlinkPolicy(final SymlinkPolicy symlinkPolicy) { + this.symlinkPolicy = Objects.requireNonNull(symlinkPolicy, "symlinkPolicy"); + return this; + } + + /** + * Writes a regular file at {@code leaf} with no-follow semantics, refusing to follow an existing symbolic link at that + * path. With overwrite disabled the open is {@code CREATE_NEW}, so an existing entry fails the extraction. + */ + void writeFile(final Path leaf, final InputStream content) throws IOException { + runBeforeLeafWrite(); + final OutputStream out = overwrite + ? Channels.newOutputStream(Files.newByteChannel(leaf, StandardOpenOption.CREATE, StandardOpenOption.WRITE, + StandardOpenOption.TRUNCATE_EXISTING, LinkOption.NOFOLLOW_LINKS)) + : Channels.newOutputStream(Files.newByteChannel(leaf, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE, + LinkOption.NOFOLLOW_LINKS)); + try (OutputStream sink = out) { + IOUtils.copy(content, sink); + } + } + + /** + * Creates a hard link at {@code leaf} to {@code linkTarget}, which is resolved against the extraction root and rejected if + * it escapes. The target is re-resolved with {@link Path#toRealPath} and re-checked before {@link Files#createLink}, which + * closes the hard-link-through-symlink escape. A residual time-of-check-to-time-of-use window remains between that re-check + * and the link creation, the same non-atomic residual as symbolic-link creation, as {@code java.nio} exposes no + * descriptor-relative {@code linkat} for the secure implementation to use. + */ + void writeHardLink(final Path leaf, final String linkTarget) throws IOException { + final Path resolved = rootDirectory.resolve(linkTarget).normalize(); + if (!isWithinRoot(resolved)) { + throw new ArchiveException("Hard link target escapes the extraction root: %s -> %s", leaf, linkTarget); + } + // The lexical check is not enough: a target routed through a symbolic link (planted or from an earlier entry) would + // be followed by createLink and escape the root. Re-resolve on disk and re-check containment, then link the real file. + final Path real = resolved.toRealPath(); + if (!isWithinRoot(real)) { + throw new ArchiveException("Hard link target resolves outside the extraction root: %s -> %s", leaf, linkTarget); + } + Files.createLink(leaf, real); + } + + /** + * Creates a symbolic link at {@code leaf} pointing at {@code linkTarget}. Under {@link SymlinkPolicy#ALLOW_WITHIN_ROOT} a + * target that resolves outside the root is rejected; under {@link SymlinkPolicy#ALLOW_ALL} the link is created verbatim. + */ + void writeSymbolicLink(final Path leaf, final String linkTarget) throws IOException { + if (symlinkPolicy == SymlinkPolicy.ALLOW_WITHIN_ROOT) { + final Path parent = leaf.getParent(); + final Path resolved = (parent != null ? parent : rootDirectory).resolve(linkTarget).normalize(); + if (!isWithinRoot(resolved)) { + throw new ArchiveException("Symbolic link target escapes the extraction root: %s -> %s", leaf, linkTarget); + } + } + Files.createSymbolicLink(leaf, leaf.getFileSystem().getPath(linkTarget)); + } +} diff --git a/src/main/java/org/apache/commons/compress/archivers/extractor/SecureExtractor.java b/src/main/java/org/apache/commons/compress/archivers/extractor/SecureExtractor.java new file mode 100644 index 00000000000..4d8e0ae82b3 --- /dev/null +++ b/src/main/java/org/apache/commons/compress/archivers/extractor/SecureExtractor.java @@ -0,0 +1,101 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.commons.compress.archivers.extractor; + +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.nio.channels.Channels; +import java.nio.channels.SeekableByteChannel; +import java.nio.file.DirectoryStream; +import java.nio.file.Files; +import java.nio.file.LinkOption; +import java.nio.file.OpenOption; +import java.nio.file.Path; +import java.nio.file.SecureDirectoryStream; +import java.nio.file.StandardOpenOption; +import java.util.ArrayDeque; +import java.util.Deque; +import java.util.HashSet; +import java.util.Set; + +import org.apache.commons.io.IOUtils; + +/** + * Race-safe {@link Extractor} for platforms whose file system provider exposes a {@link SecureDirectoryStream}. The parent of + * each regular file is re-walked component by component from the root directory handle at write time, every component opened + * with {@link LinkOption#NOFOLLOW_LINKS}, and the file is created relative to the innermost directory handle (file + * descriptor). A third party therefore cannot win a time-of-check-to-time-of-use race by swapping a parent component for a + * symbolic link between resolution and write: the write targets the pinned directory inode, not a re-resolved path. + *

+ * Directory, symbolic-link, and hard-link creation remain path-based (the {@code java.nio.file} API exposes no + * descriptor-relative {@code mkdirat}/{@code symlinkat}/{@code linkat}); the component verification still uses no-follow + * semantics, but creation of those node types carries the documented residual TOCTOU window. Regular-file writes, the common + * and highest-volume case, are fully race-safe. + *

+ *

+ * Instances are created by {@link Extractor#newExtractor(Path)}; this type is an implementation detail and is not part of the + * public API surface. + *

+ */ +final class SecureExtractor extends Extractor { + + @SuppressWarnings("unchecked") + private static SecureDirectoryStream asSecure(final DirectoryStream stream) { + return (SecureDirectoryStream) stream; + } + + SecureExtractor(final Path rootDirectory) { + super(rootDirectory); + } + + @Override + void writeFile(final Path leaf, final InputStream content) throws IOException { + final Path relative = rootDirectory.relativize(leaf); + final int count = relative.getNameCount(); + final Path fileName = relative.getFileName(); + final Deque> handles = new ArrayDeque<>(); + try { + SecureDirectoryStream dir = asSecure(Files.newDirectoryStream(rootDirectory)); + handles.push(dir); + for (int i = 0; i < count - 1; i++) { + dir = asSecure(dir.newDirectoryStream(relative.getName(i), LinkOption.NOFOLLOW_LINKS)); + handles.push(dir); + } + runBeforeLeafWrite(); + final Set options = new HashSet<>(); + options.add(StandardOpenOption.WRITE); + options.add(LinkOption.NOFOLLOW_LINKS); + if (isOverwrite()) { + options.add(StandardOpenOption.CREATE); + options.add(StandardOpenOption.TRUNCATE_EXISTING); + } else { + options.add(StandardOpenOption.CREATE_NEW); + } + try (SeekableByteChannel channel = dir.newByteChannel(fileName, options); + OutputStream out = Channels.newOutputStream(channel)) { + IOUtils.copy(content, out); + } + } finally { + while (!handles.isEmpty()) { + handles.pop().close(); + } + } + } +} diff --git a/src/main/java/org/apache/commons/compress/archivers/extractor/SpecialFilePolicy.java b/src/main/java/org/apache/commons/compress/archivers/extractor/SpecialFilePolicy.java new file mode 100644 index 00000000000..a255632bddd --- /dev/null +++ b/src/main/java/org/apache/commons/compress/archivers/extractor/SpecialFilePolicy.java @@ -0,0 +1,38 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.commons.compress.archivers.extractor; + +/** + * Controls how special entries (block devices, character devices, FIFOs) in an archive are handled during extraction. Such + * entries are never materialized by the secure path; this policy only decides whether their presence is tolerated. + * + * @since 1.29.0 + */ +public enum SpecialFilePolicy { + + /** + * Silently ignore special entries. This is the default. + */ + SKIP, + + /** + * Fail extraction if the archive contains a special entry. + */ + REJECT +} diff --git a/src/main/java/org/apache/commons/compress/archivers/extractor/SymlinkPolicy.java b/src/main/java/org/apache/commons/compress/archivers/extractor/SymlinkPolicy.java new file mode 100644 index 00000000000..f1102a4e88a --- /dev/null +++ b/src/main/java/org/apache/commons/compress/archivers/extractor/SymlinkPolicy.java @@ -0,0 +1,57 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.commons.compress.archivers.extractor; + +/** + * Controls how symbolic-link entries in an archive are handled during extraction. For untrusted archives, {@link #REJECT} + * (the default) and {@link #SKIP} are the safe choices; {@link #ALLOW_WITHIN_ROOT} enforces only a best-effort lexical + * containment check, as described on that constant. + * + * @since 1.29.0 + */ +public enum SymlinkPolicy { + + /** + * Fail extraction if the archive contains any symbolic-link entry. This is the default and the safest option for + * untrusted archives. + */ + REJECT, + + /** + * Silently ignore symbolic-link entries; nothing is created for them. + */ + SKIP, + + /** + * Create a symbolic link only when its target lexically resolves inside the extraction root, and reject a link whose + * target escapes. This containment check is best-effort: it is evaluated lexically against the archive-declared target at + * creation time, so it does not prevent a target reached through another symbolic link, nor a chain of individually + * in-root links that escapes only once the operating system resolves them together. Extraction itself never follows these + * links (every path component is resolved with {@link java.nio.file.LinkOption#NOFOLLOW_LINKS}), so nothing is written + * outside the root while extracting; the residual risk is a created link that a later consumer follows. For untrusted + * archives prefer {@link #REJECT} (the default) or {@link #SKIP}. + */ + ALLOW_WITHIN_ROOT, + + /** + * Create symbolic links verbatim, including links whose target escapes the extraction root. Only appropriate for fully + * trusted archives. + */ + ALLOW_ALL +} diff --git a/src/main/java/org/apache/commons/compress/archivers/extractor/package-info.java b/src/main/java/org/apache/commons/compress/archivers/extractor/package-info.java new file mode 100644 index 00000000000..3bc527757b7 --- /dev/null +++ b/src/main/java/org/apache/commons/compress/archivers/extractor/package-info.java @@ -0,0 +1,31 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +/** + * Safe, symlink-resistant extraction of archives into a directory. + *

+ * {@link org.apache.commons.compress.archivers.extractor.Extractor#newExtractor(java.nio.file.Path)} returns the strongest + * implementation the platform supports: a race-safe extractor backed by {@link java.nio.file.SecureDirectoryStream} where the + * file system provides one (Linux), otherwise a best-effort extractor that still resists symlink-slip from the archive but + * cannot fully close a concurrent TOCTOU race. Every path component is resolved with {@link java.nio.file.LinkOption#NOFOLLOW_LINKS} + * and regular files are written {@code CREATE_NEW} so a planted or archived symlink is never followed. + *

+ * + * @since 1.29.0 + */ +package org.apache.commons.compress.archivers.extractor; diff --git a/src/site/xdoc/examples.xml b/src/site/xdoc/examples.xml index 5ac537fc222..878cb316370 100644 --- a/src/site/xdoc/examples.xml +++ b/src/site/xdoc/examples.xml @@ -224,6 +224,79 @@ try (InputStream fi = Files.newInputStream(Paths.get("my.tar.gz")); + +

The skeleton above trusts the archive: it follows symbolic + links and can be raced by a concurrent attacker. Starting with + Compress 1.29 the + org.apache.commons.compress.archivers.extractor.Extractor + class materializes a whole archive into a target directory + safely, resisting both symlink-slip from the archive and + concurrent symlink races.

+ +

Obtain one with Extractor.newExtractor(Path), + which returns the strongest implementation the platform + supports: a race-safe extractor backed by + java.nio.file.SecureDirectoryStream where the file + system provides one (Linux), otherwise a best-effort extractor + that still resists symlink-slip from the archive but cannot + fully close a concurrent time-of-check-to-time-of-use race.

+ + i = ... create the stream for your format, use buffering...) { + Extractor.newExtractor(targetDir).extract(i); +} +]]> + +

Defaults are conservative: symbolic-link entries are + rejected, special entries such as devices and FIFOs are + skipped, and existing files are not overwritten. The handling + of symbolic links is controlled by SymlinkPolicy:

+ + + + + + + + + + + + + + + + + + + + + + + + + + +
PolicyBehavior
REJECT (default)Fail if the archive contains any symbolic-link entry. Safest for untrusted archives.
SKIPSilently ignore symbolic-link entries.
ALLOW_WITHIN_ROOTCreate a symbolic link only when its target lexically resolves inside the root. Best-effort, see the caveat below.
ALLOW_ALLCreate symbolic links verbatim. Only for fully trusted archives.
+ + + +

ALLOW_WITHIN_ROOT is best-effort: the + containment test is lexical and evaluated at creation time, so + it cannot prevent a link reached through another link, nor a + chain of individually in-root links that escapes only once the + operating system resolves them together. Extraction itself + never follows these links, so nothing is written outside the + root while extracting; the residual risk is a created link that + a later consumer follows. For untrusted archives use + REJECT (the default) or SKIP.

+ +

Apart from 7z all formats that support writing provide a subclass of ArchiveOutputStream that can be used diff --git a/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorBasePathTest.java b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorBasePathTest.java new file mode 100644 index 00000000000..b7ed4575b46 --- /dev/null +++ b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorBasePathTest.java @@ -0,0 +1,117 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.commons.compress.archivers.extractor; + +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.dir; +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.file; +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.hardlink; +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.symlink; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.apache.commons.compress.archivers.tar.TarFile; +import org.apache.commons.compress.archivers.zip.ZipFile; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Exercises the best-effort base {@link Extractor} directly (not through the factory, which on Linux returns the secure + * subclass that overrides the file write) so the base path and the {@link TarFile} / {@link ZipFile} entry points are covered + * on every platform, including the gate platform. + */ +class ExtractorBasePathTest { + + @TempDir + Path target; + + private Extractor base() throws Exception { + return new Extractor(target.toRealPath()); + } + + @Test + void baseCreatesWithinRootSymlink() throws Exception { + final byte[] data = Fixtures.tar(dir("a"), dir("b"), symlink("a/link", "../b")); + Fixtures.extractTar(base().setSymlinkPolicy(SymlinkPolicy.ALLOW_WITHIN_ROOT), data); + assertTrue(Files.isSymbolicLink(target.resolve("a/link"))); + } + + @Test + void baseOverwriteTrueReplaces() throws Exception { + Files.write(target.resolve("a.txt"), "old".getBytes(StandardCharsets.UTF_8)); + final byte[] data = Fixtures.tar(file("a.txt", "new")); + Fixtures.extractTar(base().setOverwrite(true), data); + assertArrayEquals("new".getBytes(StandardCharsets.UTF_8), Files.readAllBytes(target.resolve("a.txt"))); + } + + @Test + void extractsViaTarFile() throws Exception { + final byte[] data = Fixtures.tar(dir("a"), file("a/b.txt", "tarfile")); + try (TarFile archive = Fixtures.openTar(data)) { + base().extract(archive); + } + assertArrayEquals("tarfile".getBytes(StandardCharsets.UTF_8), Files.readAllBytes(target.resolve("a/b.txt"))); + } + + @Test + void extractsViaZipFile() throws Exception { + final byte[] data = Fixtures.zip(dir("a"), file("a/b.txt", "zipfile")); + try (ZipFile archive = Fixtures.openZip(data)) { + base().extract(archive); + } + assertArrayEquals("zipfile".getBytes(StandardCharsets.UTF_8), Files.readAllBytes(target.resolve("a/b.txt"))); + } + + @Test + void extractsHardLinkViaTarFile() throws Exception { + final byte[] data = Fixtures.tar(file("real.txt", "data"), hardlink("link", "real.txt")); + try (TarFile archive = Fixtures.openTar(data)) { + base().extract(archive); + } + assertTrue(Files.exists(target.resolve("link"))); + } + + @Test + void extractsSymlinkViaTarFile() throws Exception { + final byte[] data = Fixtures.tar(dir("b"), symlink("link", "b")); + try (TarFile archive = Fixtures.openTar(data)) { + base().setSymlinkPolicy(SymlinkPolicy.ALLOW_WITHIN_ROOT).extract(archive); + } + assertTrue(Files.isSymbolicLink(target.resolve("link"))); + } + + @Test + void extractsSymlinkViaZipFile() throws Exception { + final byte[] data = Fixtures.zip(dir("b"), symlink("link", "b")); + try (ZipFile archive = Fixtures.openZip(data)) { + base().setSymlinkPolicy(SymlinkPolicy.ALLOW_WITHIN_ROOT).extract(archive); + } + assertTrue(Files.isSymbolicLink(target.resolve("link"))); + } + + @Test + void createsStandaloneDirectory() throws Exception { + final byte[] data = Fixtures.tar(dir("emptydir")); + Fixtures.extractTar(base(), data); + assertTrue(Files.isDirectory(target.resolve("emptydir"))); + } +} diff --git a/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorFactoryTest.java b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorFactoryTest.java new file mode 100644 index 00000000000..239a741f628 --- /dev/null +++ b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorFactoryTest.java @@ -0,0 +1,76 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.commons.compress.archivers.extractor; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.io.IOException; +import java.nio.file.DirectoryStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.SecureDirectoryStream; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Factory and platform detection: {@code newExtractor} returns the secure implementation exactly when the platform's file + * system provides a {@link SecureDirectoryStream}, and validates its target. + */ +class ExtractorFactoryTest { + + @TempDir + Path target; + + @Test + void factoryMatchesPlatformCapability() throws Exception { + final boolean secureFileSystem; + try (DirectoryStream stream = Files.newDirectoryStream(target)) { + secureFileSystem = stream instanceof SecureDirectoryStream; + } + assertEquals(secureFileSystem, Extractor.newExtractor(target) instanceof SecureExtractor); + } + + @Test + void rejectsMissingTarget() { + final Path missing = target.resolve("does-not-exist"); + assertThrows(IOException.class, () -> Extractor.newExtractor(missing)); + } + + @Test + void rejectsNonDirectoryTarget() throws Exception { + final Path file = Files.createFile(target.resolve("a-file")); + assertThrows(IOException.class, () -> Extractor.newExtractor(file)); + } + + @Test + void rejectsNullTarget() { + assertThrows(NullPointerException.class, () -> Extractor.newExtractor(null)); + } + + @Test + void settersReturnSameInstanceForChaining() throws Exception { + final Extractor extractor = Extractor.newExtractor(target); + assertSame(extractor, extractor.setSymlinkPolicy(SymlinkPolicy.SKIP)); + assertSame(extractor, extractor.setSpecialFilePolicy(SpecialFilePolicy.REJECT)); + assertSame(extractor, extractor.setOverwrite(true)); + } +} diff --git a/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorFuzzTest.java b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorFuzzTest.java new file mode 100644 index 00000000000..06ba50a4793 --- /dev/null +++ b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorFuzzTest.java @@ -0,0 +1,135 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.commons.compress.archivers.extractor; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.LinkOption; +import java.nio.file.Path; +import java.util.Random; +import java.util.stream.Stream; + +import org.apache.commons.compress.archivers.tar.TarArchiveEntry; +import org.apache.commons.compress.archivers.tar.TarArchiveInputStream; +import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream; +import org.apache.commons.compress.archivers.tar.TarConstants; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Property-based adversarial fuzz: hundreds of randomized malicious tars (traversal names, escaping symlink/hardlink targets, + * special files, mixed Unicode, deep nesting) extracted under the safe policies must never escape the extraction root and must + * fail only with {@link IOException} (never an unchecked crash). A canary file at the parent level proves no escape occurred. + * Seeds are fixed so any failure is reproducible. Jazzer-style coverage-guided fuzzing with traversal sanitizers is a CI + * addition; this gives a deterministic, dependency-free adversarial sweep. + */ +class ExtractorFuzzTest { + + private static final String[] NAMES = {"f", "d/f", "d/e/f", "../escape", "../../escape", "/abs/escape", "café", + "a/../b", "x", "d"}; + + private static final String[] TARGETS = {"../escape", "../../escape", "/etc/passwd", "d", "d/f", "x", "./y"}; + + private static final SymlinkPolicy[] SAFE_POLICIES = {SymlinkPolicy.REJECT, SymlinkPolicy.SKIP, + SymlinkPolicy.ALLOW_WITHIN_ROOT}; + + private static byte[] randomTar(final Random r) { + final ByteArrayOutputStream bos = new ByteArrayOutputStream(); + try (TarArchiveOutputStream tos = new TarArchiveOutputStream(bos, "UTF-8")) { + tos.setLongFileMode(TarArchiveOutputStream.LONGFILE_POSIX); + tos.setAddPaxHeadersForNonAsciiNames(true); + final int count = 1 + r.nextInt(6); + for (int i = 0; i < count; i++) { + final String name = NAMES[r.nextInt(NAMES.length)] + (r.nextBoolean() ? "" : "/" + i); + final TarArchiveEntry entry; + byte[] content = null; + switch (r.nextInt(6)) { + case 0: + entry = new TarArchiveEntry(name); + content = ("d" + i).getBytes(StandardCharsets.UTF_8); + entry.setSize(content.length); + break; + case 1: + entry = new TarArchiveEntry(name.endsWith("/") ? name : name + "/"); + break; + case 2: + entry = new TarArchiveEntry(name, TarConstants.LF_SYMLINK); + entry.setLinkName(TARGETS[r.nextInt(TARGETS.length)]); + break; + case 3: + entry = new TarArchiveEntry(name, TarConstants.LF_LINK); + entry.setLinkName(TARGETS[r.nextInt(TARGETS.length)]); + break; + case 4: + entry = new TarArchiveEntry(name, TarConstants.LF_CHR); + break; + default: + entry = new TarArchiveEntry(name, TarConstants.LF_FIFO); + break; + } + tos.putArchiveEntry(entry); + if (content != null) { + tos.write(content); + } + tos.closeArchiveEntry(); + } + } catch (final IOException writerRejectedName) { + return null; + } + return bos.toByteArray(); + } + + @TempDir + Path target; + + @Test + void randomAdversarialArchivesNeverEscapeRoot() throws Exception { + final byte[] canaryBytes = "canary".getBytes(StandardCharsets.UTF_8); + final Path canary = Files.write(target.resolve("canary"), canaryBytes); + for (long seed = 0; seed < 384; seed++) { + final Random r = new Random(seed); + final Path root = Files.createDirectory(target.resolve("r" + seed)); + final byte[] tar = randomTar(r); + if (tar == null) { + continue; + } + final Extractor extractor = Extractor.newExtractor(root) + .setSymlinkPolicy(SAFE_POLICIES[r.nextInt(SAFE_POLICIES.length)]) + .setSpecialFilePolicy(r.nextBoolean() ? SpecialFilePolicy.SKIP : SpecialFilePolicy.REJECT) + .setOverwrite(r.nextBoolean()); + try (TarArchiveInputStream in = new TarArchiveInputStream(new ByteArrayInputStream(tar))) { + extractor.extract(in); + } catch (final IOException refusedOrMalformed) { + // A refusal (policy/escape/parse) is an acceptable outcome; only an escape or an unchecked crash is a bug. + } + } + assertArrayEquals(canaryBytes, Files.readAllBytes(canary), "the parent-level canary must be untouched"); + final long strayLeaves; + try (Stream entries = Files.list(target)) { + strayLeaves = entries.filter(p -> !Files.isDirectory(p, LinkOption.NOFOLLOW_LINKS)).count(); + } + assertEquals(1L, strayLeaves, "no file or link escaped to the parent directory (only the canary may be present)"); + } +} diff --git a/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorHappyPathTest.java b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorHappyPathTest.java new file mode 100644 index 00000000000..6d19e49e0f2 --- /dev/null +++ b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorHappyPathTest.java @@ -0,0 +1,76 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.commons.compress.archivers.extractor; + +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.dir; +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.file; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.ByteArrayInputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.apache.commons.compress.archivers.tar.TarArchiveInputStream; +import org.apache.commons.compress.archivers.zip.ZipArchiveInputStream; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Happy-path and boundary cases: a legitimate archive extracts faithfully, and an empty archive is a no-op. Proves the + * extractor does not over-block (no false positives). + */ +class ExtractorHappyPathTest { + + @TempDir + Path target; + + @Test + void emptyTarIsNoOp() throws Exception { + final byte[] data = Fixtures.tar(); + final Extractor extractor = Extractor.newExtractor(target); + try (TarArchiveInputStream in = new TarArchiveInputStream(new ByteArrayInputStream(data))) { + extractor.extract(in); + } + try (java.util.stream.Stream entries = Files.list(target)) { + assertTrue(!entries.findAny().isPresent(), "target should remain empty"); + } + } + + @Test + void extractsNestedTarViaArchiveInputStream() throws Exception { + final byte[] data = Fixtures.tar(dir("a"), file("a/b.txt", "hello")); + final Extractor extractor = Extractor.newExtractor(target); + try (TarArchiveInputStream in = new TarArchiveInputStream(new ByteArrayInputStream(data))) { + extractor.extract(in); + } + assertArrayEquals("hello".getBytes(StandardCharsets.UTF_8), Files.readAllBytes(target.resolve("a/b.txt"))); + } + + @Test + void extractsNestedZipViaArchiveInputStream() throws Exception { + final byte[] data = Fixtures.zip(dir("a"), file("a/b.txt", "world")); + final Extractor extractor = Extractor.newExtractor(target); + try (ZipArchiveInputStream in = new ZipArchiveInputStream(new ByteArrayInputStream(data))) { + extractor.extract(in); + } + assertArrayEquals("world".getBytes(StandardCharsets.UTF_8), Files.readAllBytes(target.resolve("a/b.txt"))); + } +} diff --git a/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorHardLinkTest.java b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorHardLinkTest.java new file mode 100644 index 00000000000..56a4cbb9d06 --- /dev/null +++ b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorHardLinkTest.java @@ -0,0 +1,62 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.commons.compress.archivers.extractor; + +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.file; +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.hardlink; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.BasicFileAttributes; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Hard-link (tar {@code isLink()}) handling: a link whose target stays within the root is materialized as a hard link to the + * already-extracted file; a link whose target escapes the root is rejected. + */ +class ExtractorHardLinkTest { + + @TempDir + Path target; + + @Test + void createsHardLinkWithinRoot() throws Exception { + final byte[] data = Fixtures.tar(file("real.txt", "data"), hardlink("link", "real.txt")); + final Extractor extractor = Extractor.newExtractor(target); + Fixtures.extractTar(extractor, data); + assertArrayEquals("data".getBytes(StandardCharsets.UTF_8), Files.readAllBytes(target.resolve("link"))); + final Object realKey = Files.readAttributes(target.resolve("real.txt"), BasicFileAttributes.class).fileKey(); + final Object linkKey = Files.readAttributes(target.resolve("link"), BasicFileAttributes.class).fileKey(); + assertEquals(realKey, linkKey); + } + + @Test + void rejectsHardLinkEscapingRoot() throws Exception { + final byte[] data = Fixtures.tar(hardlink("link", "../../etc/passwd")); + final Extractor extractor = Extractor.newExtractor(target); + assertThrows(IOException.class, () -> Fixtures.extractTar(extractor, data)); + } +} diff --git a/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorMalformedNameTest.java b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorMalformedNameTest.java new file mode 100644 index 00000000000..5b6b50585d8 --- /dev/null +++ b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorMalformedNameTest.java @@ -0,0 +1,97 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.commons.compress.archivers.extractor; + +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.file; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.InvalidPathException; +import java.nio.file.Path; +import java.util.stream.Stream; + +import org.apache.commons.compress.archivers.ArchiveException; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledOnOs; +import org.junit.jupiter.api.condition.OS; +import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +/** + * A malformed entry name that cannot be turned into a {@link Path} on this platform (a zip name field preserves a NUL byte, + * unlike a NUL-terminated ustar name) must fail closed with an {@link IOException}, never escape as an unchecked + * {@link InvalidPathException}. This is the "malicious input always yields IOException, never an unchecked crash" property the + * fuzz sweep relies on. + */ +class ExtractorMalformedNameTest { + + /** A zip name field can carry an embedded NUL, which {@code Paths.get} rejects with an {@link InvalidPathException}. */ + private static final String NUL_NAME = "pre" + (char) 0 + "post"; + + @TempDir + Path target; + + @Test + void nulByteZipNameFailsClosedAsArchiveException() throws Exception { + final byte[] data = Fixtures.zip(file(NUL_NAME, "x")); + final Extractor extractor = Extractor.newExtractor(target); + final IOException ex = assertThrows(IOException.class, () -> Fixtures.extractZip(extractor, data)); + assertInstanceOf(ArchiveException.class, ex); + assertInstanceOf(InvalidPathException.class, ex.getCause()); + } + + @Test + void nulByteZipNameLeavesNothingBehind() throws Exception { + final byte[] data = Fixtures.zip(file(NUL_NAME, "x")); + final Extractor extractor = Extractor.newExtractor(target); + assertThrows(IOException.class, () -> Fixtures.extractZip(extractor, data)); + try (Stream entries = Files.list(target)) { + assertEquals(0L, entries.count(), "a rejected malformed entry must not leave anything behind"); + } + } + + @Test + void wellFormedNameStillExtracts() throws Exception { + // The malformed-name guard must not regress legitimate extraction. + Fixtures.extractZip(Extractor.newExtractor(target), Fixtures.zip(file("ok.txt", "fine"))); + assertArrayEquals("fine".getBytes(StandardCharsets.UTF_8), Files.readAllBytes(target.resolve("ok.txt"))); + } + + @ParameterizedTest + @ValueSource(strings = { "evil.txt:ads", "evil*.txt", "evil?.txt", ".txt", "evil|.txt", "evil\".txt", "\\\\localhost" }) + @EnabledOnOs(OS.WINDOWS) + void illegalWindowsNameFailsClosedAsArchiveException(final String name) throws Exception { + // Windows rejects these spellings with an InvalidPathException (a reserved character, or a UNC name without a share); + // the same chokepoint turns it into the fail-closed ArchiveException the NUL case gets, with nothing left behind. + final byte[] data = Fixtures.zip(file(name, "x")); + final Extractor extractor = Extractor.newExtractor(target); + final IOException ex = assertThrows(IOException.class, () -> Fixtures.extractZip(extractor, data)); + assertInstanceOf(ArchiveException.class, ex); + assertInstanceOf(InvalidPathException.class, ex.getCause()); + try (Stream entries = Files.list(target)) { + assertEquals(0L, entries.count(), "a rejected malformed entry must not leave anything behind"); + } + } +} diff --git a/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorOverwriteTest.java b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorOverwriteTest.java new file mode 100644 index 00000000000..d469ec79700 --- /dev/null +++ b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorOverwriteTest.java @@ -0,0 +1,82 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.commons.compress.archivers.extractor; + +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.file; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Overwrite policy. The default is fail-closed ({@code CREATE_NEW}): an existing target is never silently clobbered and an + * existing symbolic link at the target path is never followed. + */ +class ExtractorOverwriteTest { + + @TempDir + Path target; + + @Test + void failsClosedWhenTargetExists() throws Exception { + Files.write(target.resolve("a.txt"), "old".getBytes(StandardCharsets.UTF_8)); + final byte[] data = Fixtures.tar(file("a.txt", "new")); + final Extractor extractor = Extractor.newExtractor(target); + assertThrows(IOException.class, () -> Fixtures.extractTar(extractor, data)); + assertArrayEquals("old".getBytes(StandardCharsets.UTF_8), Files.readAllBytes(target.resolve("a.txt"))); + } + + @Test + void overwriteFalseDoesNotFollowExistingSymlink() throws Exception { + final Path outside = Files.createTempFile("compress-extractor-outside", ".txt"); + Files.write(outside, "secret".getBytes(StandardCharsets.UTF_8)); + Files.createSymbolicLink(target.resolve("a.txt"), outside); + final byte[] data = Fixtures.tar(file("a.txt", "attacker")); + final Extractor extractor = Extractor.newExtractor(target); + assertThrows(IOException.class, () -> Fixtures.extractTar(extractor, data)); + assertArrayEquals("secret".getBytes(StandardCharsets.UTF_8), Files.readAllBytes(outside)); + } + + @Test + void overwriteReplacesExisting() throws Exception { + Files.write(target.resolve("a.txt"), "old".getBytes(StandardCharsets.UTF_8)); + final byte[] data = Fixtures.tar(file("a.txt", "new")); + final Extractor extractor = Extractor.newExtractor(target).setOverwrite(true); + Fixtures.extractTar(extractor, data); + assertArrayEquals("new".getBytes(StandardCharsets.UTF_8), Files.readAllBytes(target.resolve("a.txt"))); + } + + @Test + void overwriteTrueStillDoesNotFollowExistingSymlink() throws Exception { + // Even with overwrite enabled the write is no-follow, so an existing symbolic link at the target is never traversed. + final Path outside = Files.createTempFile("compress-extractor-outside", ".txt"); + Files.write(outside, "secret".getBytes(StandardCharsets.UTF_8)); + Files.createSymbolicLink(target.resolve("a.txt"), outside); + final byte[] data = Fixtures.tar(file("a.txt", "attacker")); + final Extractor extractor = Extractor.newExtractor(target).setOverwrite(true); + assertThrows(IOException.class, () -> Fixtures.extractTar(extractor, data)); + assertArrayEquals("secret".getBytes(StandardCharsets.UTF_8), Files.readAllBytes(outside)); + } +} diff --git a/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorPentestTest.java b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorPentestTest.java new file mode 100644 index 00000000000..21d64df3e76 --- /dev/null +++ b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorPentestTest.java @@ -0,0 +1,141 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.commons.compress.archivers.extractor; + +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.dir; +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.file; +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.hardlink; +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.symlink; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.LinkOption; +import java.nio.file.Path; + +import org.apache.commons.compress.archivers.zip.ZipFile; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Adversarial scopes beyond the core suite: a sibling directory that has the root as a string prefix (the classic + * {@code String.startsWith} trap, defeated by component-wise {@code Path.startsWith}); a hard link routed through a multi-hop + * symlink chain; a circular symlink (must fail closed, not hang); a hard link to a directory; a zip symbolic link with an + * escaping target through the {@code ZipFile} entry point; a symbolic link whose own name escapes the root; a bare {@code ..} + * name; and a deeply nested path (no stack overflow). + */ +class ExtractorPentestTest { + + @TempDir + Path target; + + @TempDir + Path attacker; + + @Test + void circularSymlinkFailsClosed() throws Exception { + Files.createSymbolicLink(target.resolve("loop"), target.resolve("loop")); + final Extractor extractor = Extractor.newExtractor(target); + assertThrows(IOException.class, () -> Fixtures.extractTar(extractor, Fixtures.tar(hardlink("x", "loop/y")))); + } + + @Test + void hardLinkToDirectoryFailsClosed() throws Exception { + final byte[] data = Fixtures.tar(dir("d"), hardlink("x", "d")); + final Extractor extractor = Extractor.newExtractor(target); + assertThrows(IOException.class, () -> Fixtures.extractTar(extractor, data)); + } + + @Test + void rejectsBareDotDotEntryName() throws Exception { + final Extractor extractor = Extractor.newExtractor(target); + assertThrows(IOException.class, () -> Fixtures.extractTar(extractor, Fixtures.tar(file("..", "p")))); + } + + @Test + void rejectsHardLinkThroughSymlinkChain() throws Exception { + Files.write(attacker.resolve("secret"), "s".getBytes(StandardCharsets.UTF_8)); + Files.createSymbolicLink(target.resolve("b"), attacker); + Files.createSymbolicLink(target.resolve("a"), target.resolve("b")); + final Extractor extractor = Extractor.newExtractor(target); + assertThrows(IOException.class, () -> Fixtures.extractTar(extractor, Fixtures.tar(hardlink("x", "a/secret")))); + assertFalse(Files.exists(target.resolve("x"))); + } + + @Test + void rejectsSiblingDirectoryWithRootAsPrefix() throws Exception { + final Path root = Files.createDirectory(target.resolve("root")); + final Extractor extractor = Extractor.newExtractor(root); + assertThrows(IOException.class, () -> Fixtures.extractTar(extractor, Fixtures.tar(file("../root-evil/x", "p")))); + assertFalse(Files.exists(target.resolve("root-evil"))); + } + + @Test + void rejectsSymlinkWhoseNameEscapes() throws Exception { + final byte[] data = Fixtures.tar(symlink("../evil", "x")); + final Extractor extractor = Extractor.newExtractor(target).setSymlinkPolicy(SymlinkPolicy.ALLOW_ALL); + assertThrows(IOException.class, () -> Fixtures.extractTar(extractor, data)); + assertFalse(Files.exists(target.getParent().resolve("evil"), LinkOption.NOFOLLOW_LINKS)); + } + + @Test + void rejectsZipSymlinkEscapeViaZipFile() throws Exception { + final byte[] data = Fixtures.zip(symlink("link", "/etc")); + final Extractor extractor = Extractor.newExtractor(target).setSymlinkPolicy(SymlinkPolicy.ALLOW_WITHIN_ROOT); + try (ZipFile zip = Fixtures.openZip(data)) { + assertThrows(IOException.class, () -> extractor.extract(zip)); + } + assertFalse(Files.exists(target.resolve("link"), LinkOption.NOFOLLOW_LINKS)); + } + + @Test + void toleratesDeepNestingWithoutStackOverflow() throws Exception { + final StringBuilder name = new StringBuilder(); + for (int i = 0; i < 120; i++) { + name.append("a/"); + } + name.append("file.txt"); + final Extractor extractor = Extractor.newExtractor(target); + Fixtures.extractTar(extractor, Fixtures.tar(file(name.toString(), "deep"))); + assertTrue(Files.exists(target.resolve(name.toString()))); + } + + @Test + void manyLegitimateEntriesAllExtract() throws Exception { + // A large legitimate archive must extract every entry: the safe defaults must not over-block real data. + final Fixtures.Entry[] entries = new Fixtures.Entry[40]; + for (int i = 0; i < entries.length; i++) { + entries[i] = file("dir" + i % 5 + "/f" + i + ".txt", "v" + i); + } + Fixtures.extractTar(Extractor.newExtractor(target), Fixtures.tar(entries)); + for (int i = 0; i < entries.length; i++) { + assertTrue(Files.exists(target.resolve("dir" + i % 5 + "/f" + i + ".txt")), "entry " + i + " should exist"); + } + } + + @Test + void fileNameCollidingWithExistingDirectoryFailsClosed() throws Exception { + Files.createDirectory(target.resolve("collide")); + final Extractor extractor = Extractor.newExtractor(target); + assertThrows(IOException.class, () -> Fixtures.extractTar(extractor, Fixtures.tar(file("collide", "x")))); + } +} diff --git a/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorResilienceTest.java b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorResilienceTest.java new file mode 100644 index 00000000000..d433780f33c --- /dev/null +++ b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorResilienceTest.java @@ -0,0 +1,229 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.commons.compress.archivers.extractor; + +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.dir; +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.fifo; +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.file; +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.hardlink; +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.symlink; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.LinkOption; +import java.nio.file.Path; +import java.nio.file.attribute.BasicFileAttributes; +import java.util.Arrays; +import java.util.stream.Stream; + +import org.apache.commons.compress.archivers.zip.ZipFile; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Resilience and robustness cases beyond the core threat suite: legitimate-but-awkward inputs (empty, Unicode, long, dotted, + * duplicate names), idempotency and reuse, mixed entry types, partial-state containment when a later entry is rejected, and the + * fail-closed behavior of forward hard-link references and null configuration. These prove the extractor neither over-blocks + * legitimate archives nor leaks state on the unhappy paths. + */ +class ExtractorResilienceTest { + + @TempDir + Path target; + + @TempDir + Path attacker; + + private static byte[] bytes(final String s) { + return s.getBytes(StandardCharsets.UTF_8); + } + + @Test + void emptyFileEntryExtractsAsEmptyFile() throws Exception { + Fixtures.extractTar(Extractor.newExtractor(target), Fixtures.tar(file("empty", ""))); + assertTrue(Files.exists(target.resolve("empty"))); + assertEquals(0L, Files.size(target.resolve("empty"))); + } + + @Test + void unicodeEntryNameExtractsContained() throws Exception { + Fixtures.extractTar(Extractor.newExtractor(target), Fixtures.tar(file("café/ünï.txt", "hi"))); + assertArrayEquals(bytes("hi"), Files.readAllBytes(target.resolve("café/ünï.txt"))); + } + + @Test + void deeplyNestedFileCreatesAllParents() throws Exception { + Fixtures.extractTar(Extractor.newExtractor(target), Fixtures.tar(file("p/q/r/s.txt", "x"))); + assertTrue(Files.isDirectory(target.resolve("p/q/r"))); + assertTrue(Files.exists(target.resolve("p/q/r/s.txt"))); + } + + @Test + void dotSegmentNameNormalizedAndContained() throws Exception { + Fixtures.extractTar(Extractor.newExtractor(target), Fixtures.tar(file("a/./b.txt", "x"))); + assertTrue(Files.exists(target.resolve("a/b.txt"))); + assertFalse(Files.exists(target.getParent().resolve("b.txt"))); + } + + @Test + void setSymlinkPolicyNullRejected() throws Exception { + final Extractor e = Extractor.newExtractor(target); + assertThrows(NullPointerException.class, () -> e.setSymlinkPolicy(null)); + } + + @Test + void setSpecialFilePolicyNullRejected() throws Exception { + final Extractor e = Extractor.newExtractor(target); + assertThrows(NullPointerException.class, () -> e.setSpecialFilePolicy(null)); + } + + @Test + void extractorReusedForTwoArchives() throws Exception { + final Extractor e = Extractor.newExtractor(target); + Fixtures.extractTar(e, Fixtures.tar(file("a.txt", "1"))); + Fixtures.extractTar(e, Fixtures.tar(file("b.txt", "2"))); + assertTrue(Files.exists(target.resolve("a.txt"))); + assertTrue(Files.exists(target.resolve("b.txt"))); + } + + @Test + void mixedEntryTypesAllExtract() throws Exception { + final byte[] data = Fixtures.tar(dir("d"), file("d/f.txt", "x"), file("real.txt", "r"), symlink("d/link", "../real.txt"), + hardlink("h.txt", "real.txt")); + final Extractor e = Extractor.newExtractor(target).setSymlinkPolicy(SymlinkPolicy.ALLOW_WITHIN_ROOT); + Fixtures.extractTar(e, data); + assertTrue(Files.exists(target.resolve("d/f.txt"))); + assertTrue(Files.isSymbolicLink(target.resolve("d/link"))); + assertTrue(Files.exists(target.resolve("h.txt"))); + } + + @Test + void hardLinkForwardReferenceFailsClosed() throws Exception { + final byte[] data = Fixtures.tar(hardlink("link", "later.txt")); + final Extractor e = Extractor.newExtractor(target); + assertThrows(IOException.class, () -> Fixtures.extractTar(e, data)); + } + + @Test + void createdSymlinkDoesNotBlockLaterLegitWrites() throws Exception { + final byte[] data = Fixtures.tar(dir("b"), symlink("b/link", "."), file("c.txt", "ok")); + final Extractor e = Extractor.newExtractor(target).setSymlinkPolicy(SymlinkPolicy.ALLOW_WITHIN_ROOT); + Fixtures.extractTar(e, data); + assertTrue(Files.isSymbolicLink(target.resolve("b/link"))); + assertTrue(Files.exists(target.resolve("c.txt"))); + } + + @Test + void midArchiveEscapeLeavesEarlierEntriesButNoEscape() throws Exception { + final byte[] data = Fixtures.tar(file("good.txt", "ok"), file("../evil.txt", "bad")); + final Extractor e = Extractor.newExtractor(target); + assertThrows(IOException.class, () -> Fixtures.extractTar(e, data)); + assertTrue(Files.exists(target.resolve("good.txt")), "earlier valid entry written"); + assertFalse(Files.exists(target.getParent().resolve("evil.txt")), "escape blocked"); + } + + @Test + void trailingSlashNameIsDirectory() throws Exception { + Fixtures.extractTar(Extractor.newExtractor(target), Fixtures.tar(dir("onlydir"))); + assertTrue(Files.isDirectory(target.resolve("onlydir"))); + } + + @Test + void longSingleComponentNameExtracts() throws Exception { + final char[] c = new char[150]; + Arrays.fill(c, 'a'); + final String name = new String(c) + ".txt"; + Fixtures.extractTar(Extractor.newExtractor(target), Fixtures.tar(file(name, "x"))); + assertTrue(Files.exists(target.resolve(name))); + } + + @Test + void multipleHardLinksShareInode() throws Exception { + final byte[] data = Fixtures.tar(file("real.txt", "data"), hardlink("h1.txt", "real.txt"), hardlink("h2.txt", "real.txt")); + Fixtures.extractTar(Extractor.newExtractor(target), data); + final Object k0 = Files.readAttributes(target.resolve("real.txt"), BasicFileAttributes.class).fileKey(); + assertEquals(k0, Files.readAttributes(target.resolve("h1.txt"), BasicFileAttributes.class).fileKey()); + assertEquals(k0, Files.readAttributes(target.resolve("h2.txt"), BasicFileAttributes.class).fileKey()); + } + + @Test + void duplicateDirectoryEntryTolerated() throws Exception { + final byte[] data = Fixtures.tar(dir("d"), dir("d"), file("d/f.txt", "x")); + Fixtures.extractTar(Extractor.newExtractor(target), data); + assertTrue(Files.exists(target.resolve("d/f.txt"))); + } + + @Test + void zipFileSymlinkSlipBlocked() throws Exception { + final byte[] data = Fixtures.zip(dir("b"), symlink("link", "b"), file("link/evil", "x")); + final Extractor e = Extractor.newExtractor(target).setSymlinkPolicy(SymlinkPolicy.ALLOW_WITHIN_ROOT); + try (ZipFile zip = Fixtures.openZip(data)) { + assertThrows(IOException.class, () -> e.extract(zip)); + } + assertFalse(Files.exists(target.resolve("b/evil"))); + } + + @Test + void skipPolicyIgnoresZipSymlinkButExtractsRest() throws Exception { + final byte[] data = Fixtures.zip(symlink("link", "/etc"), file("real.txt", "ok")); + final Extractor e = Extractor.newExtractor(target).setSymlinkPolicy(SymlinkPolicy.SKIP); + try (ZipFile zip = Fixtures.openZip(data)) { + e.extract(zip); + } + assertFalse(Files.exists(target.resolve("link"), LinkOption.NOFOLLOW_LINKS)); + assertTrue(Files.exists(target.resolve("real.txt"))); + } + + @Test + void specialRejectStopsExtraction() throws Exception { + final byte[] data = Fixtures.tar(file("a.txt", "ok"), fifo("f"), file("b.txt", "later")); + final Extractor e = Extractor.newExtractor(target).setSpecialFilePolicy(SpecialFilePolicy.REJECT); + assertThrows(IOException.class, () -> Fixtures.extractTar(e, data)); + assertTrue(Files.exists(target.resolve("a.txt"))); + assertFalse(Files.exists(target.resolve("b.txt"))); + } + + @Test + void emptyZipViaZipFileIsNoOp() throws Exception { + final byte[] data = Fixtures.zip(); + final Extractor e = Extractor.newExtractor(target); + try (ZipFile zip = Fixtures.openZip(data)) { + e.extract(zip); + } + try (Stream s = Files.list(target)) { + assertFalse(s.findAny().isPresent(), "target should remain empty"); + } + } + + @Test + void allowAllCreatesRelativeEscapingTargetButLeafStaysInRoot() throws Exception { + final byte[] data = Fixtures.tar(symlink("escape-link", "../../../../etc/passwd")); + final Extractor e = Extractor.newExtractor(target).setSymlinkPolicy(SymlinkPolicy.ALLOW_ALL); + Fixtures.extractTar(e, data); + final Path link = target.resolve("escape-link"); + assertTrue(Files.isSymbolicLink(link)); + assertEquals(link.getFileSystem().getPath("../../../../etc/passwd"), Files.readSymbolicLink(link)); + } +} diff --git a/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorRootBoundaryTest.java b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorRootBoundaryTest.java new file mode 100644 index 00000000000..ca1e401c747 --- /dev/null +++ b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorRootBoundaryTest.java @@ -0,0 +1,195 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.commons.compress.archivers.extractor; + +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.dir; +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.file; +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.hardlink; +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.symlink; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.attribute.BasicFileAttributes; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +/** + * Root-boundary invariant: {@code newExtractor} canonicalizes the target with {@code toRealPath}, so the containment guard is + * not sensitive to how the target is spelled (trailing separator, {@code .} component, or a symlinked path). This is why the + * extractor uses {@code resolveWithinRoot} against a canonical root rather than the lexical {@code ArchiveEntry.resolveIn}, + * which compares against the parent string as given and misbehaves when that parent is not normalized. + */ +class ExtractorRootBoundaryTest { + + @TempDir + Path target; + + @Test + void rejectsTraversalWhenTargetHasTrailingSeparator() throws Exception { + final Path slashy = Paths.get(target.toString() + "/"); + final Extractor extractor = Extractor.newExtractor(slashy); + assertThrows(IOException.class, () -> Fixtures.extractTar(extractor, Fixtures.tar(file("../escape", "x")))); + assertFalse(Files.exists(target.getParent().resolve("escape"))); + } + + @Test + void rejectsTraversalWhenTargetHasDotComponent() throws Exception { + final Path dotted = Paths.get(target.toString(), "."); + final Extractor extractor = Extractor.newExtractor(dotted); + assertThrows(IOException.class, () -> Fixtures.extractTar(extractor, Fixtures.tar(file("../escape", "x")))); + assertFalse(Files.exists(target.getParent().resolve("escape"))); + } + + @Test + void canonicalizesSymlinkedRootAndStaysContained() throws Exception { + final Path real = Files.createDirectory(target.resolve("real")); + final Path link = Files.createSymbolicLink(target.resolve("link"), real); + Fixtures.extractTar(Extractor.newExtractor(link), Fixtures.tar(file("a.txt", "hi"))); + assertArrayEquals("hi".getBytes(StandardCharsets.UTF_8), Files.readAllBytes(real.resolve("a.txt"))); + final Extractor extractor = Extractor.newExtractor(link); + assertThrows(IOException.class, () -> Fixtures.extractTar(extractor, Fixtures.tar(file("../escape", "x")))); + assertFalse(Files.exists(target.resolve("escape"))); + } + + @Test + void rejectsTraversalWhenTargetIsRelativeParent() throws Exception { + // A bare ".." target is the one spelling a lexical normalize() cannot collapse, so the containment check would + // wrongly accept an escape: "..".resolve("../x").normalize() is "../../x", which still startsWith(".."). The + // toRealPath in newExtractor is what defeats that, and this test fails if it is ever dropped. Only the rejecting + // half is exercised, because a benign entry would land in the parent of the working directory; the entry name is + // distinctive so that an unexpected escape is recognizable instead of looking like a stray file. + final Path relativeParent = Paths.get(".."); + final Path root = relativeParent.toRealPath(); + assumeTrue(root.getParent() != null); + final Path escaped = root.getParent().resolve("commons-compress-escape.txt"); + final Extractor extractor = Extractor.newExtractor(relativeParent); + assertThrows(IOException.class, + () -> Fixtures.extractTar(extractor, Fixtures.tar(file("../commons-compress-escape.txt", "x")))); + assertFalse(Files.exists(escaped)); + } + + @ParameterizedTest + @ValueSource(strings = { "sub/..", "a/b/../..", "a/b/c/../../.." }) + void fileResolvingToRootIsSkipped(final String name) throws Exception { + final Object rootKey = Files.readAttributes(target, BasicFileAttributes.class).fileKey(); + Fixtures.extractTar(Extractor.newExtractor(target), Fixtures.tar(file(name, "x"))); + assertTrue(Files.isDirectory(target)); + assertEquals(rootKey, Files.readAttributes(target, BasicFileAttributes.class).fileKey()); + } + + @Test + void directoryResolvingToRootIsSkipped() throws Exception { + final Object rootKey = Files.readAttributes(target, BasicFileAttributes.class).fileKey(); + Fixtures.extractTar(Extractor.newExtractor(target), Fixtures.tar(dir("sub/.."))); + assertTrue(Files.isDirectory(target)); + assertEquals(rootKey, Files.readAttributes(target, BasicFileAttributes.class).fileKey()); + } + + @Test + void symlinkResolvingToRootIsSkipped() throws Exception { + final Object rootKey = Files.readAttributes(target, BasicFileAttributes.class).fileKey(); + final Extractor extractor = Extractor.newExtractor(target).setSymlinkPolicy(SymlinkPolicy.ALLOW_WITHIN_ROOT); + Fixtures.extractTar(extractor, Fixtures.tar(symlink("sub/..", "irrelevant"))); + assertTrue(Files.isDirectory(target)); + assertEquals(rootKey, Files.readAttributes(target, BasicFileAttributes.class).fileKey()); + } + + @Test + void siblingSharingRootNamePrefixIsRejected() throws Exception { + final Path sibling = target.resolveSibling(target.getFileName().toString() + "-sibling"); + final String escape = "../" + sibling.getFileName().toString() + "/pwned"; + final Extractor extractor = Extractor.newExtractor(target); + assertThrows(IOException.class, () -> Fixtures.extractTar(extractor, Fixtures.tar(file(escape, "x")))); + assertFalse(Files.exists(sibling.resolve("pwned"))); + } + + @Test + void pathWithParentSegmentStayingInsideRootIsWritten() throws Exception { + // The skip fires only on an exact-root resolution; a name that merely contains ".." but normalizes to a location + // inside the root is still written there. + Fixtures.extractTar(Extractor.newExtractor(target), Fixtures.tar(file("a/b/../c", "hi"))); + assertArrayEquals("hi".getBytes(StandardCharsets.UTF_8), Files.readAllBytes(target.resolve("a/c"))); + assertFalse(Files.exists(target.resolve("a/b"))); + } + + @Test + void rootResolvingEntryIsSkippedWithoutAbortingTheArchive() throws Exception { + // Skipping a root-resolving entry is per-entry, not fatal, so later entries still extract. + Fixtures.extractTar(Extractor.newExtractor(target), Fixtures.tar(file("x/..", "ignored"), file("keep.txt", "hi"))); + assertArrayEquals("hi".getBytes(StandardCharsets.UTF_8), Files.readAllBytes(target.resolve("keep.txt"))); + } + + @Test + void hardlinkResolvingToRootIsSkipped() throws Exception { + // A hard link entry has no policy gate, so the root-resolution skip is what keeps its name from targeting the root. + final Object rootKey = Files.readAttributes(target, BasicFileAttributes.class).fileKey(); + Fixtures.extractTar(Extractor.newExtractor(target), Fixtures.tar(hardlink("sub/..", "whatever"))); + assertTrue(Files.isDirectory(target)); + assertEquals(rootKey, Files.readAttributes(target, BasicFileAttributes.class).fileKey()); + } + + @Test + void fileResolvingToRootIsSkippedViaZip() throws Exception { + // Same skip, exercised through the zip path for format parity with the tar cases above. + final Object rootKey = Files.readAttributes(target, BasicFileAttributes.class).fileKey(); + Fixtures.extractZip(Extractor.newExtractor(target), Fixtures.zip(file("sub/..", "x"))); + assertTrue(Files.isDirectory(target)); + assertEquals(rootKey, Files.readAttributes(target, BasicFileAttributes.class).fileKey()); + } + + @Test + void currentDirectoryEntryIsSkipped() throws Exception { + // A "." entry is common in real tar archives and resolves to the root, so it is skipped rather than failing the + // extraction or targeting the root. + final Object rootKey = Files.readAttributes(target, BasicFileAttributes.class).fileKey(); + Fixtures.extractTar(Extractor.newExtractor(target), Fixtures.tar(dir("."))); + assertTrue(Files.isDirectory(target)); + assertEquals(rootKey, Files.readAttributes(target, BasicFileAttributes.class).fileKey()); + } + + @Test + void dotSegmentMidPathIsWritten() throws Exception { + // A "." segment in the middle of a name normalizes away; the entry is still written at its real in-root location. + Fixtures.extractTar(Extractor.newExtractor(target), Fixtures.tar(file("a/./b", "hi"))); + assertArrayEquals("hi".getBytes(StandardCharsets.UTF_8), Files.readAllBytes(target.resolve("a/b"))); + } + + @Test + void backslashInNameIsLiteralNotSeparatorOnPosix() throws Exception { + // On POSIX a backslash is an ordinary filename character, not a separator, so a name that would traverse on Windows + // stays a single in-root component here. Guards against a future change that splits on backslash. + assumeTrue(File.separatorChar == '/'); + Fixtures.extractTar(Extractor.newExtractor(target), Fixtures.tar(file("a\\..\\..\\etc", "x"))); + assertTrue(Files.exists(target.resolve("a\\..\\..\\etc"))); + assertFalse(Files.exists(target.getParent().resolve("etc"))); + } +} diff --git a/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorSecurePathTest.java b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorSecurePathTest.java new file mode 100644 index 00000000000..3d4f37fcdfd --- /dev/null +++ b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorSecurePathTest.java @@ -0,0 +1,88 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.commons.compress.archivers.extractor; + +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.dir; +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.file; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Positive and adversarial coverage of the {@link SecureExtractor} regular-file write path, which only activates on a platform + * whose file system provides a {@link java.nio.file.SecureDirectoryStream} (Linux). Each test asserts the secure subclass is in + * use, so on non-secure platforms (macOS, Windows) the cases are skipped rather than silently exercising the base path. + */ +class ExtractorSecurePathTest { + + @TempDir + Path target; + + @TempDir + Path attacker; + + @BeforeEach + void requireSecurePlatform() throws Exception { + assumeTrue(Extractor.newExtractor(target) instanceof SecureExtractor, "requires a SecureDirectoryStream platform (Linux)"); + } + + @Test + void writesNestedFileRelativeToPinnedHandle() throws Exception { + final byte[] data = Fixtures.tar(dir("a"), dir("a/b"), file("a/b/c.txt", "deep-secure")); + Fixtures.extractTar(Extractor.newExtractor(target), data); + assertArrayEquals("deep-secure".getBytes(StandardCharsets.UTF_8), Files.readAllBytes(target.resolve("a/b/c.txt"))); + } + + @Test + void rejectsWriteThroughPreExistingParentSymlink() throws Exception { + Files.createSymbolicLink(target.resolve("sub"), attacker); + final byte[] data = Fixtures.tar(file("sub/file.txt", "x")); + final Extractor extractor = Extractor.newExtractor(target); + assertThrows(IOException.class, () -> Fixtures.extractTar(extractor, data)); + assertFalse(Files.exists(attacker.resolve("file.txt"))); + } + + @Test + void overwriteReplacesThroughPinnedHandle() throws Exception { + Files.write(target.resolve("a.txt"), "old".getBytes(StandardCharsets.UTF_8)); + final byte[] data = Fixtures.tar(file("a.txt", "new")); + Fixtures.extractTar(Extractor.newExtractor(target).setOverwrite(true), data); + assertArrayEquals("new".getBytes(StandardCharsets.UTF_8), Files.readAllBytes(target.resolve("a.txt"))); + } + + @Test + void overwriteDoesNotFollowExistingSymlinkThroughPinnedHandle() throws Exception { + final Path outside = Files.write(attacker.resolve("secret"), "secret".getBytes(StandardCharsets.UTF_8)); + Files.createSymbolicLink(target.resolve("a.txt"), outside); + final byte[] data = Fixtures.tar(file("a.txt", "attacker")); + final Extractor extractor = Extractor.newExtractor(target).setOverwrite(true); + assertThrows(IOException.class, () -> Fixtures.extractTar(extractor, data)); + assertArrayEquals("secret".getBytes(StandardCharsets.UTF_8), Files.readAllBytes(outside)); + } +} diff --git a/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorSpecialFileTest.java b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorSpecialFileTest.java new file mode 100644 index 00000000000..efda753492c --- /dev/null +++ b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorSpecialFileTest.java @@ -0,0 +1,77 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.commons.compress.archivers.extractor; + +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.blockDev; +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.charDev; +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.fifo; +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.file; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.LinkOption; +import java.nio.file.Path; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Special-entry (device, FIFO) handling: skipped by default so the rest of the archive still extracts, or rejected when the + * policy demands it. The secure path never materializes such entries. + */ +class ExtractorSpecialFileTest { + + @TempDir + Path target; + + @Test + void rejectsSpecialWhenConfigured() throws Exception { + final byte[] data = Fixtures.tar(fifo("fifo0")); + final Extractor extractor = Extractor.newExtractor(target).setSpecialFilePolicy(SpecialFilePolicy.REJECT); + assertThrows(IOException.class, () -> Fixtures.extractTar(extractor, data)); + } + + @Test + void skipsSpecialByDefault() throws Exception { + final byte[] data = Fixtures.tar(charDev("cdev0"), fifo("fifo0"), file("real.txt", "ok")); + final Extractor extractor = Extractor.newExtractor(target); + Fixtures.extractTar(extractor, data); + assertFalse(Files.exists(target.resolve("cdev0"), LinkOption.NOFOLLOW_LINKS)); + assertFalse(Files.exists(target.resolve("fifo0"), LinkOption.NOFOLLOW_LINKS)); + assertTrue(Files.exists(target.resolve("real.txt"))); + } + + @Test + void skipsBlockDeviceByDefault() throws Exception { + final byte[] data = Fixtures.tar(blockDev("bdev0"), file("real.txt", "ok")); + Fixtures.extractTar(Extractor.newExtractor(target), data); + assertFalse(Files.exists(target.resolve("bdev0"), LinkOption.NOFOLLOW_LINKS)); + assertTrue(Files.exists(target.resolve("real.txt"))); + } + + @Test + void rejectsBlockDeviceWhenConfigured() throws Exception { + final byte[] data = Fixtures.tar(blockDev("bdev0")); + final Extractor extractor = Extractor.newExtractor(target).setSpecialFilePolicy(SpecialFilePolicy.REJECT); + assertThrows(IOException.class, () -> Fixtures.extractTar(extractor, data)); + } +} diff --git a/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorSymlinkPolicyTest.java b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorSymlinkPolicyTest.java new file mode 100644 index 00000000000..f2b1fa90925 --- /dev/null +++ b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorSymlinkPolicyTest.java @@ -0,0 +1,201 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.commons.compress.archivers.extractor; + +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.dir; +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.file; +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.symlink; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.LinkOption; +import java.nio.file.Path; + +import org.apache.commons.compress.archivers.zip.ZipFile; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; + +/** + * Symbolic-link policy behavior. The archive itself is trusted; these prove that the extractor controls link creation and + * never follows a created link, regardless of platform (base or secure). + */ +class ExtractorSymlinkPolicyTest { + + @TempDir + Path target; + + @Test + void allowAllCreatesEscapingLink() throws Exception { + final byte[] data = Fixtures.tar(symlink("link", "/etc")); + final Extractor extractor = Extractor.newExtractor(target).setSymlinkPolicy(SymlinkPolicy.ALLOW_ALL); + Fixtures.extractTar(extractor, data); + final Path link = target.resolve("link"); + assertTrue(Files.isSymbolicLink(link)); + assertEquals(link.getFileSystem().getPath("/etc"), Files.readSymbolicLink(link)); + } + + @Test + void allowWithinRootCreatesInRootLink() throws Exception { + final byte[] data = Fixtures.tar(dir("a"), dir("b"), symlink("a/link", "../b")); + final Extractor extractor = Extractor.newExtractor(target).setSymlinkPolicy(SymlinkPolicy.ALLOW_WITHIN_ROOT); + Fixtures.extractTar(extractor, data); + final Path link = target.resolve("a/link"); + assertTrue(Files.isSymbolicLink(link)); + assertEquals(link.getFileSystem().getPath("../b"), Files.readSymbolicLink(link)); + } + + @Test + void allowWithinRootRejectsAbsoluteEscape() throws Exception { + final byte[] data = Fixtures.tar(symlink("link", "/etc")); + final Extractor extractor = Extractor.newExtractor(target).setSymlinkPolicy(SymlinkPolicy.ALLOW_WITHIN_ROOT); + assertThrows(IOException.class, () -> Fixtures.extractTar(extractor, data)); + assertFalse(Files.exists(target.resolve("link"), LinkOption.NOFOLLOW_LINKS)); + } + + @Test + void allowWithinRootRejectsRelativeEscape() throws Exception { + final byte[] data = Fixtures.tar(symlink("link", "../../etc/passwd")); + final Extractor extractor = Extractor.newExtractor(target).setSymlinkPolicy(SymlinkPolicy.ALLOW_WITHIN_ROOT); + assertThrows(IOException.class, () -> Fixtures.extractTar(extractor, data)); + assertFalse(Files.exists(target.resolve("link"), LinkOption.NOFOLLOW_LINKS)); + } + + @Test + void rejectsSymlinkByDefaultTar() throws Exception { + final byte[] data = Fixtures.tar(symlink("link", "/etc")); + final Extractor extractor = Extractor.newExtractor(target); + assertThrows(IOException.class, () -> Fixtures.extractTar(extractor, data)); + } + + @Test + void rejectsSymlinkByDefaultZip() throws Exception { + final byte[] data = Fixtures.zip(symlink("link", "/etc")); + final Extractor extractor = Extractor.newExtractor(target); + try (ZipFile zip = Fixtures.openZip(data)) { + assertThrows(IOException.class, () -> extractor.extract(zip)); + } + } + + @Test + void skipSilentlyIgnoresSymlink() throws Exception { + final byte[] data = Fixtures.tar(symlink("link", "/etc"), file("real.txt", "ok")); + final Extractor extractor = Extractor.newExtractor(target).setSymlinkPolicy(SymlinkPolicy.SKIP); + Fixtures.extractTar(extractor, data); + assertFalse(Files.exists(target.resolve("link"), LinkOption.NOFOLLOW_LINKS)); + assertTrue(Files.exists(target.resolve("real.txt"))); + } + + @Test + void allowWithinRootCreatesZipSymlinkViaZipFile() throws Exception { + final byte[] data = Fixtures.zip(dir("a"), dir("b"), symlink("a/link", "../b")); + final Extractor extractor = Extractor.newExtractor(target).setSymlinkPolicy(SymlinkPolicy.ALLOW_WITHIN_ROOT); + try (ZipFile zip = Fixtures.openZip(data)) { + extractor.extract(zip); + } + final Path link = target.resolve("a/link"); + assertTrue(Files.isSymbolicLink(link)); + assertEquals(link.getFileSystem().getPath("../b"), Files.readSymbolicLink(link)); + } + + private static final int POC_DEPTH = 20; + + private static String repeat(final String component, final int times) { + final StringBuilder sb = new StringBuilder(component); + for (int i = 1; i < times; i++) { + sb.append('/').append(component); + } + return sb.toString(); + } + + /** + * Builds the indirect (chained) symbolic-link escape from COMPRESS-727: {@code malicious-entry} is written first with a + * target that is lexically inside the root, while the deeper link it passes through only escapes once the operating system + * resolves the chain. Each link is individually within-root, which is exactly what {@link SymlinkPolicy#ALLOW_WITHIN_ROOT} + * cannot see through. + */ + private static byte[] indirectEscapeZip() throws IOException { + final String nested = repeat("a", POC_DEPTH); + final String maliciousTarget = nested + "/" + repeat("..", POC_DEPTH) + "/etc/passwd"; + final String nestedTarget = repeat("..", POC_DEPTH - 1); + return Fixtures.zip(symlink("malicious-entry", maliciousTarget), symlink(nested, nestedTarget)); + } + + @Test + void indirectSymlinkEscapeRejectedByDefault() throws Exception { + final Extractor extractor = Extractor.newExtractor(target); + try (ZipFile zip = Fixtures.openZip(indirectEscapeZip())) { + assertThrows(IOException.class, () -> extractor.extract(zip)); + } + assertFalse(Files.exists(target.resolve("malicious-entry"), LinkOption.NOFOLLOW_LINKS)); + } + + @Test + void indirectSymlinkEscapeSkipped() throws Exception { + final Extractor extractor = Extractor.newExtractor(target).setSymlinkPolicy(SymlinkPolicy.SKIP); + try (ZipFile zip = Fixtures.openZip(indirectEscapeZip())) { + extractor.extract(zip); + } + assertFalse(Files.exists(target.resolve("malicious-entry"), LinkOption.NOFOLLOW_LINKS)); + } + + @ParameterizedTest + @EnumSource(names = { "ALLOW_WITHIN_ROOT", "ALLOW_ALL" }) + void indirectSymlinkEscapeCreatesInRootLinksButWritesNothingOutside(final SymlinkPolicy policy) throws Exception { + final String nested = repeat("a", POC_DEPTH); + final String maliciousTarget = nested + "/" + repeat("..", POC_DEPTH) + "/etc/passwd"; + final String nestedTarget = repeat("..", POC_DEPTH - 1); + final byte[] data = Fixtures.zip(symlink("malicious-entry", maliciousTarget), symlink(nested, nestedTarget)); + final Extractor extractor = Extractor.newExtractor(target).setSymlinkPolicy(policy); + try (ZipFile zip = Fixtures.openZip(data)) { + extractor.extract(zip); + } + final Path malicious = target.resolve("malicious-entry"); + final Path deep = target.resolve(nested); + // Each link passes the per-link lexical containment check, so both are created inside the root. This is the + // documented best-effort limitation: the escape only occurs if a later consumer follows the chain, never during + // extraction, which resolves no links. + assertTrue(Files.isSymbolicLink(malicious)); + assertTrue(Files.isSymbolicLink(deep)); + // The crafted targets are stored verbatim (the fixture is not silently neutered) and only links, no file content, were + // written, so extraction produced nothing outside the root. + assertEquals(malicious.getFileSystem().getPath(maliciousTarget), Files.readSymbolicLink(malicious)); + assertEquals(deep.getFileSystem().getPath(nestedTarget), Files.readSymbolicLink(deep)); + } + + @Test + void legitSymlinkChainWithinRootIsCreated() throws Exception { + // Positive control for the chain the escape test attacks: when every hop stays inside the root the whole chain is + // created, so ALLOW_WITHIN_ROOT is not over-rejecting legitimate links. + final byte[] data = Fixtures.zip(dir("b"), symlink("a", "b"), symlink("c", "a")); + final Extractor extractor = Extractor.newExtractor(target).setSymlinkPolicy(SymlinkPolicy.ALLOW_WITHIN_ROOT); + try (ZipFile zip = Fixtures.openZip(data)) { + extractor.extract(zip); + } + assertTrue(Files.isSymbolicLink(target.resolve("a"))); + assertTrue(Files.isSymbolicLink(target.resolve("c"))); + assertEquals(target.getFileSystem().getPath("b"), Files.readSymbolicLink(target.resolve("a"))); + assertEquals(target.getFileSystem().getPath("a"), Files.readSymbolicLink(target.resolve("c"))); + } +} diff --git a/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorSymlinkSlipTest.java b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorSymlinkSlipTest.java new file mode 100644 index 00000000000..0c18ba9de17 --- /dev/null +++ b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorSymlinkSlipTest.java @@ -0,0 +1,95 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.commons.compress.archivers.extractor; + +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.dir; +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.file; +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.hardlink; +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.symlink; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Symlink-slip and duplicate-entry hardening: writes must never traverse a symbolic link in a parent component, whether the + * link was planted on disk beforehand or created by an earlier archive entry, and a duplicate entry must fail closed rather + * than overwrite. These are the generic defensive properties that also defeat normalization/case-collision variants, since + * the defense is a file-system-level no-follow check on the resolved inode, not a lexical name comparison. + */ +class ExtractorSymlinkSlipTest { + + @TempDir + Path target; + + @TempDir + Path attacker; + + @Test + void duplicateEntryFailsClosed() throws Exception { + final byte[] data = Fixtures.tar(file("a.txt", "first"), file("a.txt", "second")); + final Extractor extractor = Extractor.newExtractor(target); + assertThrows(IOException.class, () -> Fixtures.extractTar(extractor, data)); + assertArrayEquals("first".getBytes(StandardCharsets.UTF_8), Files.readAllBytes(target.resolve("a.txt"))); + } + + @Test + void neverFollowsCreatedSymlinkForLaterWrite() throws Exception { + final byte[] data = Fixtures.tar(dir("b"), symlink("link", "b"), file("link/evil", "x")); + final Extractor extractor = Extractor.newExtractor(target).setSymlinkPolicy(SymlinkPolicy.ALLOW_WITHIN_ROOT); + assertThrows(IOException.class, () -> Fixtures.extractTar(extractor, data)); + assertFalse(Files.exists(target.resolve("b/evil"))); + } + + @Test + void rejectsWriteThroughPreExistingParentSymlink() throws Exception { + Files.createSymbolicLink(target.resolve("sub"), attacker); + final byte[] data = Fixtures.tar(file("sub/file.txt", "x")); + final Extractor extractor = Extractor.newExtractor(target); + assertThrows(IOException.class, () -> Fixtures.extractTar(extractor, data)); + assertFalse(Files.exists(attacker.resolve("file.txt"))); + } + + @Test + void baseRejectsWriteThroughPreExistingParentSymlink() throws Exception { + Files.createSymbolicLink(target.resolve("sub"), attacker); + final byte[] data = Fixtures.tar(file("sub/file.txt", "x")); + final Extractor extractor = new Extractor(target.toRealPath()); + assertThrows(IOException.class, () -> Fixtures.extractTar(extractor, data)); + assertFalse(Files.exists(attacker.resolve("file.txt"))); + } + + @Test + void rejectsHardLinkTargetThroughSymlink() throws Exception { + // node-tar CVE-2026-26960 class: a hard link whose target routes through a symlink must not escape the root. + Files.write(attacker.resolve("secret"), "secret".getBytes(StandardCharsets.UTF_8)); + Files.createSymbolicLink(target.resolve("s"), attacker); + final byte[] data = Fixtures.tar(hardlink("x", "s/secret")); + final Extractor extractor = Extractor.newExtractor(target); + assertThrows(IOException.class, () -> Fixtures.extractTar(extractor, data)); + assertFalse(Files.exists(target.resolve("x"))); + } +} diff --git a/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorZipSlipTest.java b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorZipSlipTest.java new file mode 100644 index 00000000000..a0d8671f642 --- /dev/null +++ b/src/test/java/org/apache/commons/compress/archivers/extractor/ExtractorZipSlipTest.java @@ -0,0 +1,148 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.commons.compress.archivers.extractor; + +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.file; +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.rawFile; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.stream.Stream; + +import org.apache.commons.compress.archivers.ArchiveException; +import org.apache.commons.compress.archivers.tar.TarArchiveInputStream; +import org.apache.commons.compress.archivers.zip.ZipFile; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledOnOs; +import org.junit.jupiter.api.condition.OS; +import org.junit.jupiter.api.function.Executable; +import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +/** + * Zip-slip regression: entry names that escape the target via {@code ..} or an absolute path must be rejected, preserving the + * lexical guard that {@code ArchiveEntry.resolveIn} provides today. The guard decides containment from the name alone, so a UNC + * or drive name on Windows has to fail with the guard's own exception and never with a {@code FileSystemException} from an API + * that touched the name first. + */ +class ExtractorZipSlipTest { + + /** A UNC path on Windows and an absolute path everywhere else; the guard rejects it lexically on both. */ + private static final String HOST_SHARE_NAME = "//localhost/invalid/evil.txt"; + + @TempDir + Path target; + + /** + * Asserts that the extraction fails with the zip-slip guard's own exception and leaves the target empty. A + * {@code FileSystemException} here ("The network name cannot be found" for a UNC name) would mean an API touched the name + * before containment was decided. + */ + private void assertRejectedByGuard(final Executable extraction) throws IOException { + final ArchiveException ex = assertThrows(ArchiveException.class, extraction); + assertTrue(ex.getMessage().contains("would escape the extraction root"), () -> "expected the zip-slip guard, got: " + ex); + assertNull(ex.getCause(), () -> "the zip-slip guard throws without a cause, got: " + ex.getCause()); + try (Stream entries = Files.list(target)) { + assertEquals(0L, entries.count(), "a rejected entry must not leave anything behind"); + } + } + + @Test + void rejectsAbsoluteName() throws Exception { + final byte[] data = Fixtures.tar(rawFile("/tmp/compress-extractor-evil", "x")); + final Extractor extractor = Extractor.newExtractor(target); + try (TarArchiveInputStream in = new TarArchiveInputStream(new ByteArrayInputStream(data))) { + assertThrows(IOException.class, () -> extractor.extract(in)); + } + assertFalse(Files.exists(Paths.get("/tmp/compress-extractor-evil"))); + } + + @Test + void rejectsParentTraversal() throws Exception { + final byte[] data = Fixtures.tar(file("../compress-extractor-evil.txt", "x")); + final Extractor extractor = Extractor.newExtractor(target); + try (TarArchiveInputStream in = new TarArchiveInputStream(new ByteArrayInputStream(data))) { + assertThrows(IOException.class, () -> extractor.extract(in)); + } + assertFalse(Files.exists(target.getParent().resolve("compress-extractor-evil.txt"))); + } + + @Test + void rejectsHostShareNameViaZipFile() throws Exception { + final byte[] data = Fixtures.zip(file(HOST_SHARE_NAME, "x")); + final Extractor extractor = Extractor.newExtractor(target); + try (ZipFile zip = Fixtures.openZip(data)) { + assertRejectedByGuard(() -> extractor.extract(zip)); + } + } + + @Test + void rejectsHostShareNameViaZipStream() throws Exception { + final byte[] data = Fixtures.zip(file(HOST_SHARE_NAME, "x")); + final Extractor extractor = Extractor.newExtractor(target); + assertRejectedByGuard(() -> Fixtures.extractZip(extractor, data)); + } + + @Test + void rejectsHostShareNameViaTar() throws Exception { + final byte[] data = Fixtures.tar(rawFile(HOST_SHARE_NAME, "x")); + final Extractor extractor = Extractor.newExtractor(target); + assertRejectedByGuard(() -> Fixtures.extractTar(extractor, data)); + } + + @ParameterizedTest + @ValueSource(strings = { "\\\\localhost\\invalid", "\\\\localhost\\invalid\\evil.txt", "\\\\localhost\\invalid/evil.txt", + "//localhost/invalid/evil.txt", "C:\\Windows\\evil.txt", "C:/Windows/evil.txt" }) + @EnabledOnOs(OS.WINDOWS) + void rejectsUncOrDriveNameOnWindows(final String name) throws Exception { + // Each spelling is a UNC or absolute drive path to WindowsPathParser, so the guard rejects it from the name alone. A + // FileSystemException ("The network name cannot be found") would mean the name reached the file system, and possibly + // a remote host, before containment was decided. The zip layer folds a backslash-only name to forward slashes, which + // Windows reads as UNC just the same. + final byte[] data = Fixtures.zip(file(name, "x")); + final Extractor extractor = Extractor.newExtractor(target); + try (ZipFile zip = Fixtures.openZip(data)) { + assertRejectedByGuard(() -> extractor.extract(zip)); + } + assertRejectedByGuard(() -> Fixtures.extractZip(Extractor.newExtractor(target), data)); + } + + @Test + @EnabledOnOs(OS.WINDOWS) + void rejectsDriveRelativeNameOnAnotherDriveOnWindows() throws Exception { + // "Q:evil.txt" is relative to the current directory of drive Q:, which only the file system (GetFullPathName) can + // supply. The guard rejects it from the spelling alone, so that drive is never consulted. + final char rootDrive = Character.toUpperCase(target.toRealPath().getRoot().toString().charAt(0)); + final String name = (rootDrive == 'Q' ? "R" : "Q") + ":evil.txt"; + final byte[] data = Fixtures.zip(file(name, "x")); + final Extractor extractor = Extractor.newExtractor(target); + try (ZipFile zip = Fixtures.openZip(data)) { + assertRejectedByGuard(() -> extractor.extract(zip)); + } + } +} diff --git a/src/test/java/org/apache/commons/compress/archivers/extractor/Fixtures.java b/src/test/java/org/apache/commons/compress/archivers/extractor/Fixtures.java new file mode 100644 index 00000000000..11e6e02d625 --- /dev/null +++ b/src/test/java/org/apache/commons/compress/archivers/extractor/Fixtures.java @@ -0,0 +1,208 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.commons.compress.archivers.extractor; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.apache.commons.compress.archivers.tar.TarArchiveEntry; +import org.apache.commons.compress.archivers.tar.TarArchiveInputStream; +import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream; +import org.apache.commons.compress.archivers.tar.TarConstants; +import org.apache.commons.compress.archivers.tar.TarFile; +import org.apache.commons.compress.archivers.zip.UnixStat; +import org.apache.commons.compress.archivers.zip.ZipArchiveEntry; +import org.apache.commons.compress.archivers.zip.ZipArchiveInputStream; +import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream; +import org.apache.commons.compress.archivers.zip.ZipFile; + +/** + * Builds in-memory tar and zip archives with arbitrary entry types (regular, directory, symbolic link, hard link, special) + * for extractor tests. Entries are written with the public archive writers, which faithfully encode link flags and unix + * modes. {@link Entry#rawFile} preserves a leading {@code /} (the writer strips it by default), so an absolute-path escape can + * be exercised. Duplicate-name and collision fixtures (not used in this public suite) would need low-level construction. + */ +final class Fixtures { + + enum Kind { + FILE, DIR, SYMLINK, HARDLINK, CHARDEV, BLOCKDEV, FIFO + } + + static final class Entry { + + static Entry blockDev(final String name) { + return new Entry(Kind.BLOCKDEV, name, null, false); + } + + static Entry charDev(final String name) { + return new Entry(Kind.CHARDEV, name, null, false); + } + + static Entry dir(final String name) { + return new Entry(Kind.DIR, name, null, false); + } + + static Entry fifo(final String name) { + return new Entry(Kind.FIFO, name, null, false); + } + + static Entry file(final String name, final String content) { + return new Entry(Kind.FILE, name, content, false); + } + + static Entry hardlink(final String name, final String target) { + return new Entry(Kind.HARDLINK, name, target, false); + } + + static Entry rawFile(final String name, final String content) { + return new Entry(Kind.FILE, name, content, true); + } + + static Entry symlink(final String name, final String target) { + return new Entry(Kind.SYMLINK, name, target, false); + } + + final Kind kind; + final String name; + final String data; + final boolean preserveAbsolute; + + private Entry(final Kind kind, final String name, final String data, final boolean preserveAbsolute) { + this.kind = kind; + this.name = name; + this.data = data; + this.preserveAbsolute = preserveAbsolute; + } + } + + static void extractTar(final Extractor extractor, final byte[] tar) throws IOException { + try (TarArchiveInputStream in = new TarArchiveInputStream(new ByteArrayInputStream(tar))) { + extractor.extract(in); + } + } + + static void extractZip(final Extractor extractor, final byte[] zip) throws IOException { + try (ZipArchiveInputStream in = new ZipArchiveInputStream(new ByteArrayInputStream(zip))) { + extractor.extract(in); + } + } + + static byte[] tar(final Entry... entries) throws IOException { + final ByteArrayOutputStream bos = new ByteArrayOutputStream(); + try (TarArchiveOutputStream tos = new TarArchiveOutputStream(bos)) { + tos.setLongFileMode(TarArchiveOutputStream.LONGFILE_POSIX); + for (final Entry e : entries) { + final TarArchiveEntry te; + byte[] content = null; + switch (e.kind) { + case FILE: + te = e.preserveAbsolute ? new TarArchiveEntry(e.name, TarConstants.LF_NORMAL, true) : new TarArchiveEntry(e.name); + content = e.data.getBytes(StandardCharsets.UTF_8); + te.setSize(content.length); + break; + case DIR: + te = new TarArchiveEntry(e.name.endsWith("/") ? e.name : e.name + "/"); + break; + case SYMLINK: + te = new TarArchiveEntry(e.name, TarConstants.LF_SYMLINK); + te.setLinkName(e.data); + break; + case HARDLINK: + te = new TarArchiveEntry(e.name, TarConstants.LF_LINK); + te.setLinkName(e.data); + break; + case CHARDEV: + te = new TarArchiveEntry(e.name, TarConstants.LF_CHR); + break; + case BLOCKDEV: + te = new TarArchiveEntry(e.name, TarConstants.LF_BLK); + break; + case FIFO: + te = new TarArchiveEntry(e.name, TarConstants.LF_FIFO); + break; + default: + throw new IllegalArgumentException(String.valueOf(e.kind)); + } + tos.putArchiveEntry(te); + if (content != null) { + tos.write(content); + } + tos.closeArchiveEntry(); + } + } + return bos.toByteArray(); + } + + static byte[] zip(final Entry... entries) throws IOException { + final ByteArrayOutputStream bos = new ByteArrayOutputStream(); + try (ZipArchiveOutputStream zos = new ZipArchiveOutputStream(bos)) { + for (final Entry e : entries) { + final ZipArchiveEntry ze; + byte[] content = null; + switch (e.kind) { + case FILE: + ze = new ZipArchiveEntry(e.name); + content = e.data.getBytes(StandardCharsets.UTF_8); + break; + case DIR: + ze = new ZipArchiveEntry(e.name.endsWith("/") ? e.name : e.name + "/"); + break; + case SYMLINK: + ze = new ZipArchiveEntry(e.name); + ze.setUnixMode(UnixStat.LINK_FLAG | 0777); + content = e.data.getBytes(StandardCharsets.UTF_8); + break; + default: + throw new IllegalArgumentException("zip fixture does not support " + e.kind); + } + zos.putArchiveEntry(ze); + if (content != null) { + zos.write(content); + } + zos.closeArchiveEntry(); + } + } + return bos.toByteArray(); + } + + static TarFile openTar(final byte[] tar) throws IOException { + final Path file = Files.createTempFile("compress-extractor-tar", ".tar"); + Files.write(file, tar); + return TarFile.builder().setPath(file).get(); + } + + static ZipFile openZip(final byte[] zip) throws IOException { + final Path file = Files.createTempFile("compress-extractor-zip", ".zip"); + Files.write(file, zip); + return ZipFile.builder().setPath(file).get(); + } + + static Path write(final Path dir, final String name, final byte[] data) throws IOException { + final Path p = dir.resolve(name); + Files.write(p, data); + return p; + } + + private Fixtures() { + } +} diff --git a/src/test/java/org/apache/commons/compress/archivers/extractor/SecureExtractorRaceTest.java b/src/test/java/org/apache/commons/compress/archivers/extractor/SecureExtractorRaceTest.java new file mode 100644 index 00000000000..e0b1d10d519 --- /dev/null +++ b/src/test/java/org/apache/commons/compress/archivers/extractor/SecureExtractorRaceTest.java @@ -0,0 +1,87 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.commons.compress.archivers.extractor; + +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.dir; +import static org.apache.commons.compress.archivers.extractor.Fixtures.Entry.file; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Concurrent TOCTOU race: a third party swaps a parent directory component for a symbolic link after the extractor has + * resolved the parent but before the leaf file is written (driven deterministically through the {@code beforeLeafWrite} test + * seam). The secure extractor, which writes relative to a pinned directory handle, must not let the write escape; the base + * extractor, which re-resolves the path, follows the swap (the documented residual). + */ +class SecureExtractorRaceTest { + + @TempDir + Path target; + + @TempDir + Path attacker; + + private static byte[] entryUnderSub() throws IOException { + return Fixtures.tar(dir("sub"), file("sub/file.txt", "payload")); + } + + private Runnable swapSubForSymlink(final Path root) { + return () -> { + try { + final Path sub = root.resolve("sub"); + Files.delete(sub); + Files.createSymbolicLink(sub, attacker); + } catch (final IOException e) { + throw new UncheckedIOException(e); + } + }; + } + + @Test + void baseExtractorDoesNotDefeatParentSwap() throws Exception { + final Path root = target.toRealPath(); + final Extractor extractor = new Extractor(root); + extractor.setBeforeLeafWrite(swapSubForSymlink(root)); + Fixtures.extractTar(extractor, entryUnderSub()); + assertTrue(Files.exists(attacker.resolve("file.txt")), "base extractor follows the swapped parent (documented residual)"); + } + + @Test + void secureExtractorDefeatsParentSwap() throws Exception { + final Path root = target.toRealPath(); + final Extractor extractor = Extractor.newExtractor(target); + assumeTrue(extractor instanceof SecureExtractor, "requires a SecureDirectoryStream platform (Linux)"); + extractor.setBeforeLeafWrite(swapSubForSymlink(root)); + try { + Fixtures.extractTar(extractor, entryUnderSub()); + } catch (final IOException failClosed) { + // Fail-closed (the pinned parent was unlinked) is an acceptable secure outcome; the escape must not happen. + } + assertFalse(Files.exists(attacker.resolve("file.txt")), "secure extractor must not write through the swapped parent"); + } +}