diff options
2 files changed, 35 insertions, 2 deletions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ZipStreamReader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ZipStreamReader.java index 40d53271908..5188652d269 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ZipStreamReader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ZipStreamReader.java @@ -7,6 +7,8 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.UncheckedIOException; +import java.nio.file.Path; +import java.util.Arrays; import java.util.List; import java.util.function.Predicate; import java.util.zip.ZipEntry; @@ -26,7 +28,7 @@ public class ZipStreamReader { ImmutableList.Builder<ZipEntryWithContent> builder = new ImmutableList.Builder<>(); ZipEntry zipEntry; while (null != (zipEntry = zipInput.getNextEntry())) { - if (!entryNameMatcher.test(zipEntry.getName())) continue; + if (!entryNameMatcher.test(requireName(zipEntry.getName()))) continue; builder.add(new ZipEntryWithContent(zipEntry, readContent(zipInput))); } entries = builder.build(); @@ -55,7 +57,14 @@ public class ZipStreamReader { } public List<ZipEntryWithContent> entries() { return entries; } - + + private static String requireName(String name) { + IllegalArgumentException e = new IllegalArgumentException("Unexpected non-normalized path found in zip content"); + if (Arrays.asList(name.split("/")).contains("..")) throw e; + if (!name.equals(Path.of(name).normalize().toString())) throw e; + return name; + } + public static class ZipEntryWithContent { private final ZipEntry zipEntry; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/ZipStreamReaderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/ZipStreamReaderTest.java index fb7622d3078..92462a67e48 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/ZipStreamReaderTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/ZipStreamReaderTest.java @@ -13,6 +13,8 @@ import java.util.zip.ZipEntry; import java.util.zip.ZipOutputStream; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; /** @@ -36,6 +38,28 @@ public class ZipStreamReaderTest { assertEquals("foobar", new String(extracted, StandardCharsets.UTF_8)); } + @Test + public void test_paths() { + Map<String, Boolean> tests = Map.of( + "../../services.xml", true, + "/../.././services.xml", true, + "./application/././services.xml", true, + "application//services.xml", true, + "services..xml", false, + "application/services.xml", false, + "components/foo-bar-deploy.jar", false, + "services.xml", false + ); + tests.forEach((name, expectException) -> { + try { + new ZipStreamReader(new ByteArrayInputStream(zip(Map.of(name, "foo"))), name::equals, 1024); + assertFalse("Expected exception for '" + name + "'", expectException); + } catch (IllegalArgumentException ignored) { + assertTrue("Unexpected exception for '" + name + "'", expectException); + } + }); + } + private static byte[] zip(Map<String, String> entries) { ByteArrayOutputStream zip = new ByteArrayOutputStream(); try (ZipOutputStream out = new ZipOutputStream(zip)) { |