This is an automated email from the ASF dual-hosted git repository. ggregory pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-compress.git
commit 3d80677a86615888f3f0808e7c46f3f66bf5869e Author: Gary Gregory <garydgreg...@gmail.com> AuthorDate: Wed Jan 17 08:32:44 2024 -0500 Add and use ArchiveEntry.resolveIn(Path) --- src/changes/changes.xml | 7 ++++--- .../commons/compress/archivers/ArchiveEntry.java | 20 ++++++++++++++++++++ .../compress/archivers/ArchiveException.java | 10 +++++----- .../compress/archivers/examples/Expander.java | 9 ++------- .../org/apache/commons/compress/AbstractTest.java | 21 +++++++++------------ .../commons/compress/archivers/zip/Lister.java | 17 ++++++++++------- 6 files changed, 50 insertions(+), 34 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index c458e6bd2..c0819d2e4 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -75,9 +75,10 @@ The <action> type attribute can be add,update,fix,remove. <action type="fix" dev="ggregory" due-to="Gary Gregory">Deprecate ByteUtils.fromLittleEndian(InputStream, int).</action> <action type="fix" dev="ggregory" due-to="Gary Gregory">Deprecate ByteUtils.toLittleEndian(DataOutput, long, int).</action> <!-- ADD --> - <action type="add" dev="ggregory" due-to="Gary Gregory">Add ZipFile.builder(), add ZipFile.Builder, deprecate constructors.</action> - <action type="add" dev="ggregory" due-to="Gary Gregory">Add SevenZFile.builder(), add SevenZFile.Builder, deprecate constructors.</action> - <action type="add" dev="ggregory" due-to="Gary Gregory">Add ArchiveInputStream.getCharset().</action> + <action type="add" dev="ggregory" due-to="Gary Gregory">Add and use ZipFile.builder(), ZipFile.Builder, and deprecate constructors.</action> + <action type="add" dev="ggregory" due-to="Gary Gregory">Add and use SevenZFile.builder(), SevenZFile.Builder, and deprecate constructors.</action> + <action type="add" dev="ggregory" due-to="Gary Gregory">Add and use ArchiveInputStream.getCharset().</action> + <action type="add" dev="ggregory" due-to="Gary Gregory">Add and use ArchiveEntry.resolveIn(Path).</action> <!-- UPDATE --> <action type="update" dev="ggregory" due-to="Gary Gregory">Bump commons-lang3 from 3.13.0 to 3.14.0.</action> <action type="update" dev="ggregory" due-to="Dependabot">Bump com.github.marschall:memoryfilesystem from 2.6.1 to 2.8.0 #444, #458.</action> diff --git a/src/main/java/org/apache/commons/compress/archivers/ArchiveEntry.java b/src/main/java/org/apache/commons/compress/archivers/ArchiveEntry.java index 2015f76e1..d329868c2 100644 --- a/src/main/java/org/apache/commons/compress/archivers/ArchiveEntry.java +++ b/src/main/java/org/apache/commons/compress/archivers/ArchiveEntry.java @@ -18,6 +18,8 @@ */ package org.apache.commons.compress.archivers; +import java.io.IOException; +import java.nio.file.Path; import java.util.Date; /** @@ -62,4 +64,22 @@ public interface ArchiveEntry { * @return true if this entry refers to a directory. */ boolean isDirectory(); + + /** + * Resolves this entry in the given parent Path. + * + * @param parentPath the {@link Path#resolve(Path)} receiver. + * @return a resolved and normalized Path. + * @throws IOException if this method detects a Zip slip. + * @since 1.26.0 + */ + default Path resolveIn(final Path parentPath) throws IOException { + final String name = getName(); + final Path outputFile = parentPath.resolve(name).normalize(); + if (!outputFile.startsWith(parentPath)) { + throw new IOException(String.format("Zip slip '%s' + '%s' -> '%s'", parentPath, name, outputFile)); + } + return outputFile; + } + } diff --git a/src/main/java/org/apache/commons/compress/archivers/ArchiveException.java b/src/main/java/org/apache/commons/compress/archivers/ArchiveException.java index 1c2248ccf..de22b5725 100644 --- a/src/main/java/org/apache/commons/compress/archivers/ArchiveException.java +++ b/src/main/java/org/apache/commons/compress/archivers/ArchiveException.java @@ -19,17 +19,17 @@ package org.apache.commons.compress.archivers; /** - * Archiver related Exception + * Archiver related Exception. */ public class ArchiveException extends Exception { - /** Serial */ + /** Serial. */ private static final long serialVersionUID = 2772690708123267100L; /** * Constructs a new exception with the specified detail message. The cause is not initialized. * - * @param message the detail message + * @param message the detail message. */ public ArchiveException(final String message) { super(message); @@ -38,8 +38,8 @@ public class ArchiveException extends Exception { /** * Constructs a new exception with the specified detail message and cause. * - * @param message the detail message - * @param cause the cause + * @param message the detail message. + * @param cause the cause. */ public ArchiveException(final String message, final Exception cause) { super(message, cause); diff --git a/src/main/java/org/apache/commons/compress/archivers/examples/Expander.java b/src/main/java/org/apache/commons/compress/archivers/examples/Expander.java index 29ad6ec83..bd91a8336 100644 --- a/src/main/java/org/apache/commons/compress/archivers/examples/Expander.java +++ b/src/main/java/org/apache/commons/compress/archivers/examples/Expander.java @@ -41,7 +41,7 @@ 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.compress.utils.IOUtils; +import org.apache.commons.io.IOUtils; import org.apache.commons.io.output.NullOutputStream; /** @@ -70,12 +70,7 @@ public class Expander { final Path targetDirPath = nullTarget ? null : targetDirectory.normalize(); T nextEntry = supplier.get(); while (nextEntry != null) { - final Path targetPath = nullTarget ? null : targetDirectory.resolve(nextEntry.getName()); - // check if targetDirectory and f are the same path - this may - // happen if the nextEntry.getName() is "./" - if (!nullTarget && !targetPath.normalize().startsWith(targetDirPath) && !Files.isSameFile(targetDirectory, targetPath)) { - throw new IOException("Expanding " + nextEntry.getName() + " would create file outside of " + targetDirectory); - } + final Path targetPath = nullTarget ? null : nextEntry.resolveIn(targetDirPath); if (nextEntry.isDirectory()) { if (!nullTarget && !Files.isDirectory(targetPath) && Files.createDirectories(targetPath) == null) { throw new IOException("Failed to create directory " + targetPath); diff --git a/src/test/java/org/apache/commons/compress/AbstractTest.java b/src/test/java/org/apache/commons/compress/AbstractTest.java index a4070ebe9..1772b81ca 100644 --- a/src/test/java/org/apache/commons/compress/AbstractTest.java +++ b/src/test/java/org/apache/commons/compress/AbstractTest.java @@ -32,7 +32,6 @@ import java.net.URISyntaxException; import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -152,27 +151,25 @@ public abstract class AbstractTest extends AbstractTempDirTest { * @throws Exception */ protected File checkArchiveContent(final ArchiveInputStream<?> inputStream, final List<String> expected, final boolean cleanUp) throws Exception { - final Path result = createTempDirectory("dir-result"); + final Path targetDir = createTempDirectory("dir-result"); + final Path result = targetDir.resolve("result"); try { ArchiveEntry entry; while ((entry = inputStream.getNextEntry()) != null) { - final Path output = Paths.get(result.toString(), "result", entry.getName()).normalize(); - if (!output.startsWith(result)) { - throw new IOException("Zip slip " + output); - } + final Path outputFile = entry.resolveIn(result); long bytesCopied = 0; if (entry.isDirectory()) { - Files.createDirectories(output); + Files.createDirectories(outputFile); } else { - Files.createDirectories(output.getParent()); - bytesCopied = Files.copy(inputStream, output); + Files.createDirectories(outputFile.getParent()); + bytesCopied = Files.copy(inputStream, outputFile); } final long size = entry.getSize(); if (size != ArchiveEntry.SIZE_UNKNOWN) { assertEquals(size, bytesCopied, "Entry.size should equal bytes read."); } - if (!Files.exists(output)) { + if (!Files.exists(outputFile)) { fail("Extraction failed: " + entry.getName()); } if (expected != null && !expected.remove(getExpectedString(entry))) { @@ -188,10 +185,10 @@ public abstract class AbstractTest extends AbstractTempDirTest { } } finally { if (cleanUp) { - forceDelete(result); + forceDelete(targetDir); } } - return result.toFile(); + return targetDir.toFile(); } /** diff --git a/src/test/java/org/apache/commons/compress/archivers/zip/Lister.java b/src/test/java/org/apache/commons/compress/archivers/zip/Lister.java index 7c4192435..ef4f9a11b 100644 --- a/src/test/java/org/apache/commons/compress/archivers/zip/Lister.java +++ b/src/test/java/org/apache/commons/compress/archivers/zip/Lister.java @@ -22,6 +22,8 @@ import java.io.File; import java.io.IOException; import java.io.InputStream; import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.Enumeration; import org.apache.commons.compress.archivers.ArchiveEntry; @@ -42,15 +44,16 @@ public final class Lister { boolean useStream = false; String encoding; boolean allowStoredEntriesWithDataDescriptor = false; - String dir; + Path dir; } - private static void extract(final String dirName, final ZipArchiveEntry entry, final InputStream inputStream) throws IOException { - final File file = new File(dirName, entry.getName()); - if (!file.getParentFile().exists()) { - file.getParentFile().mkdirs(); + private static void extract(final Path targetDir, final ZipArchiveEntry entry, final InputStream inputStream) throws IOException { + final Path outputFile = entry.resolveIn(targetDir); + final Path parent = outputFile.getParent(); + if (parent != null && !Files.exists(parent)) { + Files.createDirectories(parent); } - Files.copy(inputStream, file.toPath()); + Files.copy(inputStream, outputFile); } private static void list(final ZipArchiveEntry entry) { @@ -104,7 +107,7 @@ public final class Lister { } } else if (args[i].equals("-extract")) { if (argsLength > i + 1) { - cl.dir = args[++i]; + cl.dir = Paths.get(args[++i]); } else { System.err.println("missing argument to -extract"); error = true;