Reject non-normalized names of zip file entries

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)) {