Check filenames inside of archives.

src/extension/wps/wps-core/pom.xml

@@ -130,6 +130,13 @@
            <classifier>tests</classifier>
            <scope>test</scope>
         </dependency>
+        <dependency>
+           <groupId>org.geoserver</groupId>
+           <artifactId>gs-platform</artifactId>
+           <version>${gs.version}</version>
+           <classifier>tests</classifier>
+           <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>org.geoserver</groupId>
             <artifactId>gs-wcs2_0</artifactId>

src/extension/wps/wps-core/src/main/java/org/geoserver/wps/ppio/ShapeZipPPIO.java

@@ -72,8 +72,7 @@ public Object decode(InputStream input) throws Exception {
             ZipEntry entry = null;
 
             while ((entry = zis.getNextEntry()) != null) {
-                String name = entry.getName();
-                File file = new File(tempDir, entry.getName());
+                File file = IOUtils.getZipOutputFile(tempDir, entry);
                 if (entry.isDirectory()) {
                     file.mkdir();
                 } else {

src/extension/wps/wps-core/src/test/java/org/geoserver/wps/ppio/ShapeZipPPIOTest.java

@@ -0,0 +1,25 @@
+/* (c) 2018 Open Source Geospatial Foundation - all rights reserved
+ * This code is licensed under the GPL 2.0 license, available at the root
+ * application directory.
+ */
+package org.geoserver.wps.ppio;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.io.IOException;
+import java.io.InputStream;
+import org.junit.Test;
+
+public class ShapeZipPPIOTest {
+
+    @Test
+    public void testDecodeBadEntryName() throws Exception {
+        try (InputStream input = getClass().getResourceAsStream("/bad-zip-file.zip")) {
+            new ShapeZipPPIO(null).decode(input);
+            fail("Expected decompression to fail");
+        } catch (IOException e) {
+            assertTrue(e.getMessage().startsWith("Entry is outside of the target directory"));
+        }
+    }
+}

src/platform/src/main/java/org/geoserver/util/IOUtils.java

@@ -324,13 +324,32 @@ private static void zipDirectory(
         zipout.flush();
     }
 
+    /**
+     * Gets the output file for the provided zip entry and checks that it will not be written
+     * outside of the target directory.
+     *
+     * @param destDir the output directory
+     * @param entry the zip entry
+     * @return the output file
+     * @throws IOException if the zip entry is outside of the target directory
+     */
+    public static File getZipOutputFile(File destDir, ZipEntry entry) throws IOException {
+        String canonicalDirectory = destDir.getCanonicalPath();
+        File file = new File(destDir, entry.getName());
+        String canonicalFile = file.getCanonicalPath();
+        if (canonicalFile.startsWith(canonicalDirectory + File.separator)) {
+            return file;
+        }
+        throw new IOException("Entry is outside of the target directory: " + entry.getName());
+    }
+
     public static void decompress(InputStream input, File destDir) throws IOException {
         ZipInputStream zin = new ZipInputStream(input);
         ZipEntry entry = null;
 
         byte[] buffer = new byte[1024];
         while ((entry = zin.getNextEntry()) != null) {
-            File f = new File(destDir, entry.getName());
+            File f = getZipOutputFile(destDir, entry);
             if (entry.isDirectory()) {
                 f.mkdirs();
                 continue;
@@ -349,41 +368,38 @@ public static void decompress(InputStream input, File destDir) throws IOExceptio
     }
 
     public static void decompress(final File inputFile, final File destDir) throws IOException {
-        ZipFile zipFile = new ZipFile(inputFile);
-
-        Enumeration<? extends ZipEntry> entries = zipFile.entries();
-
-        while (entries.hasMoreElements()) {
-            ZipEntry entry = (ZipEntry) entries.nextElement();
-            InputStream stream = zipFile.getInputStream(entry);
-
-            if (entry.isDirectory()) {
-                // Assume directories are stored parents first then children.
-                (new File(destDir, entry.getName())).mkdir();
-                continue;
-            }
+        try (ZipFile zipFile = new ZipFile(inputFile)) {
+            Enumeration<? extends ZipEntry> entries = zipFile.entries();
+
+            while (entries.hasMoreElements()) {
+                ZipEntry entry = entries.nextElement();
+                File newFile = getZipOutputFile(destDir, entry);
+                if (entry.isDirectory()) {
+                    // Assume directories are stored parents first then children.
+                    newFile.mkdir();
+                    continue;
+                }
 
-            File newFile = new File(destDir, entry.getName());
-            FileOutputStream fos = new FileOutputStream(newFile);
-            try {
-                byte[] buf = new byte[1024];
-                int len;
+                InputStream stream = zipFile.getInputStream(entry);
+                FileOutputStream fos = new FileOutputStream(newFile);
+                try {
+                    byte[] buf = new byte[1024];
+                    int len;
 
-                while ((len = stream.read(buf)) >= 0) saveCompressedStream(buf, fos, len);
+                    while ((len = stream.read(buf)) >= 0) saveCompressedStream(buf, fos, len);
 
-            } catch (IOException e) {
-                zipFile.close();
-                IOException ioe = new IOException("Not valid COAMPS archive file type.");
-                ioe.initCause(e);
-                throw ioe;
-            } finally {
-                fos.flush();
-                fos.close();
+                } catch (IOException e) {
+                    IOException ioe = new IOException("Not valid archive file type.");
+                    ioe.initCause(e);
+                    throw ioe;
+                } finally {
+                    fos.flush();
+                    fos.close();
 
-                stream.close();
+                    stream.close();
+                }
             }
         }
-        zipFile.close();
     }
 
     /**

src/platform/src/test/java/org/geoserver/util/IOUtilsTest.java

@@ -6,12 +6,14 @@
 package org.geoserver.util;
 
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import com.google.common.io.Files;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.IOException;
+import java.io.InputStream;
 import java.nio.file.Path;
 import java.util.zip.ZipOutputStream;
 import org.junit.Rule;
@@ -41,4 +43,29 @@ public void testZipUnzip() throws IOException {
 
         assertTrue(p2.resolve("foo/bar/bar.txt").toFile().exists());
     }
+
+    @Test
+    public void testDecompressStreamBadEntryName() throws IOException {
+        File destDir = temp.newFolder("d3").toPath().toFile();
+        destDir.mkdirs();
+        try (InputStream input = getClass().getResourceAsStream("/bad-zip-file.zip")) {
+            IOUtils.decompress(input, destDir);
+            fail("Expected decompression to fail");
+        } catch (IOException e) {
+            assertTrue(e.getMessage().startsWith("Entry is outside of the target directory"));
+        }
+    }
+
+    @Test
+    public void testDecompressFileBadEntryName() throws IOException {
+        File destDir = temp.newFolder("d4").toPath().toFile();
+        destDir.mkdirs();
+        File input = new File("src/test/resources/bad-zip-file.zip");
+        try {
+            IOUtils.decompress(input, destDir);
+            fail("Expected decompression to fail");
+        } catch (IOException e) {
+            assertTrue(e.getMessage().startsWith("Entry is outside of the target directory"));
+        }
+    }
 }

src/rest/pom.xml

@@ -35,6 +35,13 @@
       <classifier>tests</classifier>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.geoserver</groupId>
+      <artifactId>gs-platform</artifactId>
+      <version>${project.version}</version>
+      <classifier>tests</classifier>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.springframework</groupId>
       <artifactId>spring-test</artifactId>

src/rest/src/main/java/org/geoserver/rest/util/IOUtils.java

@@ -764,9 +764,10 @@ public static void inflate(
 
         final Enumeration<? extends ZipEntry> entries = archive.entries();
         try {
+            File destDir = outputDirectory.dir();
             while (entries.hasMoreElements()) {
                 ZipEntry entry = entries.nextElement();
-
+                org.geoserver.util.IOUtils.getZipOutputFile(destDir, entry);
                 if (!entry.isDirectory()) {
                     final String name = entry.getName();
                     final String ext = FilenameUtils.getExtension(name);

src/rest/src/main/java/org/geoserver/rest/util/RESTUtils.java

@@ -6,7 +6,6 @@
 package org.geoserver.rest.util;
 
 import java.io.File;
-import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -19,9 +18,7 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.logging.Logger;
-import java.util.zip.ZipEntry;
 import java.util.zip.ZipFile;
-import java.util.zip.ZipInputStream;
 import javax.servlet.http.HttpServletRequest;
 import org.apache.commons.io.FilenameUtils;
 import org.geoserver.catalog.Catalog;
@@ -463,29 +460,6 @@ public static void remapping(
      * @throws IOException
      */
     public static void unzipInputStream(InputStream in, File outputDirectory) throws IOException {
-        ZipInputStream zin = null;
-
-        try {
-            zin = new ZipInputStream(in);
-
-            ZipEntry entry;
-            byte[] buffer = new byte[2048];
-
-            while ((entry = zin.getNextEntry()) != null) {
-                String outpath = outputDirectory.getAbsolutePath() + "/" + entry.getName();
-                FileOutputStream output = null;
-                try {
-                    output = new FileOutputStream(outpath);
-                    int len;
-                    while ((len = zin.read(buffer)) > 0) {
-                        output.write(buffer, 0, len);
-                    }
-                } finally {
-                    IOUtils.closeQuietly(output);
-                }
-            }
-        } finally {
-            IOUtils.closeQuietly(zin);
-        }
+        org.geoserver.util.IOUtils.decompress(in, outputDirectory);
     }
 }

src/rest/src/test/java/org/geoserver/rest/util/IOUtilsTest.java

@@ -0,0 +1,39 @@
+/* (c) 2018 Open Source Geospatial Foundation - all rights reserved
+ * This code is licensed under the GPL 2.0 license, available at the root
+ * application directory.
+ */
+package org.geoserver.rest.util;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.zip.ZipFile;
+import org.geoserver.platform.GeoServerResourceLoader;
+import org.geoserver.platform.resource.Resource;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+public class IOUtilsTest {
+
+    @Rule public TemporaryFolder temp = new TemporaryFolder(new File("target"));
+
+    @Test
+    public void testInflateBadEntryName() throws IOException {
+        File destDir = temp.newFolder("d1").toPath().toFile();
+        destDir.mkdirs();
+        Resource directory = new GeoServerResourceLoader(destDir).get("");
+        try (InputStream input = getClass().getResourceAsStream("/bad-zip-file.zip")) {
+            File file = new File(destDir, "bad-zip-file.zip");
+            IOUtils.copyStream(input, new FileOutputStream(file), false, true);
+            IOUtils.inflate(new ZipFile(file), directory, null, null, null, null, false, false);
+            fail("Expected decompression to fail");
+        } catch (IOException e) {
+            assertTrue(e.getMessage().startsWith("Entry is outside of the target directory"));
+        }
+    }
+}