[GEOS-11176] Add validation to file wrapper resource paths (#7234)

Co-authored-by: Steve Ikeoka <steve.ikeoka@gdit.com>
geoserver · Oct 30, 2023 · fe235b3 · fe235b3
1 parent 53ae090
commit fe235b3
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 1 deletion.
diff --git a/src/platform/src/main/java/org/geoserver/platform/resource/Files.java b/src/platform/src/main/java/org/geoserver/platform/resource/Files.java
@@ -14,6 +14,7 @@
 import java.io.OutputStream;
 import java.net.URL;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
@@ -47,9 +48,28 @@ static final class ResourceAdaptor implements Resource {
         final File file;
 
         private ResourceAdaptor(File file) {
+            valid(file.getPath());
             this.file = file.getAbsoluteFile();
         }
 
+        /**
+         * GeoServer unit tests heavily utilize single period path components (e.g., ./something or
+         * target/./something) and Windows path separators when running on Windows so this method is
+         * a more lenient version of {@link Paths#valid(String)} that only checks for double period
+         * path components to prevent path traversal vulnerabilities.
+         *
+         * @param path the file path
+         * @return the file path
+         * @throws IllegalArgumentException If path contains '..'
+         */
+        private static String valid(String path) {
+            if (path != null
+                    && Arrays.stream(Paths.convert(path).split("/")).anyMatch(".."::equals)) {
+                throw new IllegalArgumentException("Contains invalid '..' path: " + path);
+            }
+            return path;
+        }
+
         @Override
         public String path() {
             return Paths.convert(file.getPath());
@@ -199,7 +219,7 @@ public Resource parent() {
 
         @Override
         public Resource get(String resourcePath) {
-            return new ResourceAdaptor(new File(file, resourcePath));
+            return new ResourceAdaptor(new File(file, valid(resourcePath)));
         }
 
         @Override

diff --git a/src/platform/src/main/java/org/geoserver/platform/resource/URIs.java b/src/platform/src/main/java/org/geoserver/platform/resource/URIs.java
@@ -28,6 +28,7 @@ static class ResourceAdaptor implements Resource {
         private URL url;
 
         public ResourceAdaptor(URL url) {
+            Paths.valid(url.getPath());
             this.url = url;
         }
 

diff --git a/src/platform/src/test/java/org/geoserver/platform/resource/ResourceTheoryTest.java b/src/platform/src/test/java/org/geoserver/platform/resource/ResourceTheoryTest.java
@@ -32,7 +32,9 @@
 import java.util.Collection;
 import java.util.List;
 import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.SystemUtils;
 import org.geoserver.platform.resource.Resource.Type;
+import org.junit.Test;
 import org.junit.experimental.theories.Theories;
 import org.junit.experimental.theories.Theory;
 import org.junit.runner.RunWith;
@@ -490,4 +492,24 @@ public void theoryRootIsAbsolute(String path) throws Exception {
         final Resource res = getResource(Paths.convert(file.getPath()));
         assertTrue(Paths.isAbsolute(res.path()));
     }
+
+    @Test
+    public void testPathTraversal() {
+        assertInvalidPath("..");
+        assertInvalidPath("../foo/bar");
+        assertInvalidPath("foo/../bar");
+        assertInvalidPath("foo/bar/..");
+    }
+
+    @Test
+    public void testPathTraversalWindows() {
+        assumeTrue(SystemUtils.IS_OS_WINDOWS);
+        assertInvalidPath("..\\foo\\bar");
+        assertInvalidPath("foo\\..\\bar");
+        assertInvalidPath("foo\\bar\\..");
+    }
+
+    protected final void assertInvalidPath(String path) {
+        assertThrows(IllegalArgumentException.class, () -> getResource(path));
+    }
 }