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..c972c06bfa4 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.validateArchiveEntry(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..c4ab5782ce9 100644 --- a/api/src/main/java/ai/djl/util/ZipUtils.java +++ b/api/src/main/java/ai/djl/util/ZipUtils.java @@ -52,10 +52,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 = validateArchiveEntry(entry.getName(), dest); set.add(name); Path file = dest.resolve(name).toAbsolutePath(); if (entry.isDirectory()) { @@ -121,14 +118,16 @@ private static void addToZip(Path root, Path file, ZipOutputStream zos) throws I } } - static String removeLeadingFileSeparator(String name) { - int index = 0; - for (; index < name.length(); index++) { - if (name.charAt(index) != File.separatorChar) { - break; - } + static String validateArchiveEntry(String name, Path destination) throws IOException { + Path expectedOutputPath = destination.resolve(name).normalize(); + if (!expectedOutputPath.startsWith(destination.normalize())) { + throw new IOException( + "Bad archive entry " + + name + + ". Attempted write outside destination " + + destination); } - return name.substring(index); + return name; } private static final class ValidationInputStream extends FilterInputStream { diff --git a/api/src/test/java/ai/djl/util/ZipUtilsTest.java b/api/src/test/java/ai/djl/util/ZipUtilsTest.java index 387715bbd44..e8b896c6b7a 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,20 @@ 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); try (InputStream is = Files.newInputStream(path)) { - TarUtils.untar(is, output, false); + Assert.assertThrows(() -> TarUtils.untar(is, output, false)); + } + } + + @Test + public void testLinuxCreatedWindowsUsedOffendingTar() throws IOException { + TestRequirements.windows(); + Path tarPath = Paths.get("src/test/resources/linux_create_windows_use.tar"); + Path output = Paths.get("C:/out"); + try (InputStream is = Files.newInputStream(tarPath)) { + Assert.assertThrows(() -> TarUtils.untar(is, output, false)); } - Assert.assertTrue(Files.exists(file)); } @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"))) {