diff --git a/api/build.gradle.kts b/api/build.gradle.kts index 025f546c994..9a6a259e39b 100644 --- a/api/build.gradle.kts +++ b/api/build.gradle.kts @@ -15,6 +15,7 @@ dependencies { exclude("junit", "junit") } testImplementation(libs.slf4j.simple) + testImplementation(project(":testing")) testRuntimeOnly(project(":engines:pytorch:pytorch-model-zoo")) testRuntimeOnly(project(":engines:pytorch:pytorch-jni")) } diff --git a/api/src/main/java/ai/djl/util/TarUtils.java b/api/src/main/java/ai/djl/util/TarUtils.java index d4a6e42b230..fedd1fb71d3 100644 --- a/api/src/main/java/ai/djl/util/TarUtils.java +++ b/api/src/main/java/ai/djl/util/TarUtils.java @@ -48,10 +48,7 @@ public static void untar(InputStream is, Path dir, boolean gzip) throws IOExcept try (TarArchiveInputStream tis = new TarArchiveInputStream(bis)) { TarArchiveEntry entry; while ((entry = tis.getNextEntry()) != null) { - String entryName = ZipUtils.removeLeadingFileSeparator(entry.getName()); - if (entryName.contains("..")) { - throw new IOException("Malicious zip entry: " + entryName); - } + String entryName = ZipUtils.sanitizeAndValidateArchiveEntry(entry.getName(), dir); Path file = dir.resolve(entryName).toAbsolutePath(); if (entry.isDirectory()) { Files.createDirectories(file); diff --git a/api/src/main/java/ai/djl/util/ZipUtils.java b/api/src/main/java/ai/djl/util/ZipUtils.java index 7c8c298a6cb..a5a77b28b1e 100644 --- a/api/src/main/java/ai/djl/util/ZipUtils.java +++ b/api/src/main/java/ai/djl/util/ZipUtils.java @@ -12,6 +12,8 @@ */ package ai.djl.util; +import org.apache.commons.io.FilenameUtils; + import java.io.BufferedOutputStream; import java.io.ByteArrayOutputStream; import java.io.File; @@ -52,10 +54,7 @@ public static void unzip(InputStream is, Path dest) throws IOException { ZipEntry entry; Set set = new HashSet<>(); while ((entry = zis.getNextEntry()) != null) { - String name = removeLeadingFileSeparator(entry.getName()); - if (name.contains("..")) { - throw new IOException("Malicious zip entry: " + name); - } + String name = sanitizeAndValidateArchiveEntry(entry.getName(), dest); set.add(name); Path file = dest.resolve(name).toAbsolutePath(); if (entry.isDirectory()) { @@ -121,13 +120,31 @@ private static void addToZip(Path root, Path file, ZipOutputStream zos) throws I } } + static String sanitizeAndValidateArchiveEntry(String name, Path destination) + throws IOException { + String sanitizedName = removeLeadingFileSeparator(name); + Path expectedOutputPath = destination.resolve(sanitizedName).normalize(); + if (!expectedOutputPath.startsWith(destination.normalize())) { + throw new IOException( + "Bad archive entry " + + name + + ". Attempted write outside destination " + + destination); + } + return sanitizedName; + } + static String removeLeadingFileSeparator(String name) { + String osAwareArchiveEntryName = FilenameUtils.separatorsToSystem(name); int index = 0; - for (; index < name.length(); index++) { - if (name.charAt(index) != File.separatorChar) { + for (; index < osAwareArchiveEntryName.length(); index++) { + if (osAwareArchiveEntryName.charAt(index) != File.separatorChar) { break; } } + // We return the substring of the original entry here because for zip archives + // we validate this entry against the header and do not want to modify the + // separator char as that can cause false positive issues in ValidationInputStream logic return name.substring(index); } diff --git a/api/src/test/java/ai/djl/util/ZipUtilsTest.java b/api/src/test/java/ai/djl/util/ZipUtilsTest.java index 387715bbd44..d686eb6373f 100644 --- a/api/src/test/java/ai/djl/util/ZipUtilsTest.java +++ b/api/src/test/java/ai/djl/util/ZipUtilsTest.java @@ -12,6 +12,8 @@ */ package ai.djl.util; +import ai.djl.testing.TestRequirements; + import org.apache.commons.compress.archivers.zip.Zip64Mode; import org.apache.commons.compress.archivers.zip.ZipArchiveEntry; import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream; @@ -49,13 +51,26 @@ public void testEmptyZipFile() throws IOException { public void testOffendingTar() throws IOException { Path path = Paths.get("src/test/resources/offending.tar"); Path output = Paths.get("build/output"); - Path file = output.resolve("tmp/empty.txt"); - Utils.deleteQuietly(file); Files.createDirectories(output); + Path outputFile = output.resolve("tmp/empty.txt"); + Utils.deleteQuietly(outputFile); try (InputStream is = Files.newInputStream(path)) { TarUtils.untar(is, output, false); } - Assert.assertTrue(Files.exists(file)); + Assert.assertTrue(Files.exists(outputFile)); + } + + @Test + public void testLinuxCreatedWindowsUsedOffendingTar() throws IOException { + TestRequirements.windows(); + Path tarPath = Paths.get("src/test/resources/linux_create_windows_use.tar"); + Path output = Paths.get("build/output"); + Files.createDirectories(output); + Path outputFile = output.resolve("Windows/System32/drivers/etc/dummy.txt"); + try (InputStream is = Files.newInputStream(tarPath)) { + TarUtils.untar(is, output, false); + } + Assert.assertTrue(Files.exists(outputFile)); } @Test diff --git a/api/src/test/resources/linux_create_windows_use.tar b/api/src/test/resources/linux_create_windows_use.tar new file mode 100644 index 00000000000..757e28c0044 Binary files /dev/null and b/api/src/test/resources/linux_create_windows_use.tar differ diff --git a/integration/src/test/java/ai/djl/integration/IntegrationTests.java b/integration/src/test/java/ai/djl/integration/IntegrationTests.java index 23267808fd7..b1f4d991d9f 100644 --- a/integration/src/test/java/ai/djl/integration/IntegrationTests.java +++ b/integration/src/test/java/ai/djl/integration/IntegrationTests.java @@ -38,7 +38,7 @@ public void runIntegrationTests() { // TODO: windows CPU build is having OOM issue if 3 engines are loaded and running tests // together if (System.getProperty("os.name").startsWith("Win")) { - engines.add("MXNet"); + engines.add("PyTorch"); } else if ("aarch64".equals(System.getProperty("os.arch"))) { engines.add("PyTorch"); } else { diff --git a/testing/src/main/java/ai/djl/testing/TestRequirements.java b/testing/src/main/java/ai/djl/testing/TestRequirements.java index d39d7be9dfc..32a507cd1a9 100644 --- a/testing/src/main/java/ai/djl/testing/TestRequirements.java +++ b/testing/src/main/java/ai/djl/testing/TestRequirements.java @@ -96,6 +96,13 @@ public static void notWindows() { } } + /** Requires that the test runs on windows, not OSX or linux. */ + public static void windows() { + if (!System.getProperty("os.name").toLowerCase().startsWith("win")) { + throw new SkipException("This test requires windows"); + } + } + /** Requires that the test runs on x86_64 arch. */ public static void notArm() { if ("aarch64".equals(System.getProperty("os.arch"))) {