Disallow relative paths and specs

2 files changed, 52 insertions, 10 deletions

diff --git a/container-core/src/main/java/com/yahoo/restapi/Path.java b/container-core/src/main/java/com/yahoo/restapi/Path.java
index 00a47e6ddf2..b96488c6781 100644
--- a/container-core/src/main/java/com/yahoo/restapi/Path.java
+++ b/container-core/src/main/java/com/yahoo/restapi/Path.java
@@ -7,10 +7,12 @@ import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Objects;
+import java.util.function.Function;
 import java.util.stream.Stream;
 
 /**
- * A path which is able to match strings containing bracketed placeholders and return the
+ * A normalized path which is able to match strings containing bracketed placeholders and return the
  * values given at the placeholders. The path is split on '/', and each part is then URL decoded.
  * 
  * E.g a path /a/1/bar/fuz/baz/%62%2f
@@ -22,8 +24,8 @@ import java.util.stream.Stream;
  * If the path spec ends with /{*}, it will match urls with any rest path.
  * The rest path (not including the trailing slash) will be available as getRest().
  * 
- * Note that for convenience in common use this has state which changes as a side effect of each matches
- * invocation. It is therefore for single thread use.
+ * Note that for convenience in common use this has state which changes as a side effect of each
+ * {@link Path#matches(String)} invocation. It is therefore for single thread use.
  *
  * @author bratseth
  */
@@ -38,15 +40,13 @@ public class Path {
     private String rest = "";
 
     public Path(URI uri) {
-        this.pathString = uri.getRawPath();
-        this.elements = Stream.of(this.pathString.split("/"))
-                              .map(part -> URLDecoder.decode(part, StandardCharsets.UTF_8))
-                              .toArray(String[]::new);
+        this.pathString = requireNormalized(uri).getRawPath();
+        this.elements = splitAbsolutePath(pathString, (part) -> URLDecoder.decode(part, StandardCharsets.UTF_8));
     }
 
     private boolean matchesInner(String pathSpec) {
         values.clear();
-        String[] specElements = pathSpec.split("/");
+        String[] specElements = splitAbsolutePath(pathSpec, Function.identity());
         boolean matchPrefix = false;
         if (specElements.length > 1 && specElements[specElements.length-1].equals("{*}")) {
             matchPrefix = true;
@@ -88,7 +88,7 @@ public class Path {
      *
      * This will NOT match empty path elements.
      *
-     * @param pathSpec the path string to match to this
+     * @param pathSpec the literal path string to match to this
      * @return true if the string matches, false otherwise
      */
     public boolean matches(String pathSpec) {
@@ -117,5 +117,21 @@ public class Path {
     public String toString() {
         return "path '" + String.join("/", elements) + "'";
     }
+
+    private static URI requireNormalized(URI uri) {
+        Objects.requireNonNull(uri);
+        if (!uri.normalize().equals(uri)) throw new IllegalArgumentException("Expected normalized URI, got '" + uri + "'");
+        return uri;
+    }
+
+    private static String[] splitAbsolutePath(String path, Function<String, String> partParser) {
+        String[] parts = Stream.of(path.split("/"))
+                               .map(partParser)
+                               .toArray(String[]::new);
+        for (var part : parts) {
+            if (part.equals("..")) throw new IllegalArgumentException("Expected absolute path, got '" + path + "'");
+        }
+        return parts;
+    }
     
 }
diff --git a/container-core/src/test/java/com/yahoo/restapi/PathTest.java b/container-core/src/test/java/com/yahoo/restapi/PathTest.java
index 0deb27ae0f2..5cbf80ff2ad 100644
--- a/container-core/src/test/java/com/yahoo/restapi/PathTest.java
+++ b/container-core/src/test/java/com/yahoo/restapi/PathTest.java
@@ -8,6 +8,7 @@ import java.net.URI;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
 
 /**
  * @author bratseth
@@ -64,8 +65,8 @@ public class PathTest {
     @Test
     public void testUrlEncodedPath() {
         assertTrue(new Path(URI.create("/a/%62/c")).matches("/a/b/c"));
-        assertTrue(new Path(URI.create("/a/%2e%2e/c")).matches("/a/../c"));
         assertFalse(new Path(URI.create("/a/b%2fc")).matches("/a/b/c"));
+        assertFalse(new Path(URI.create("/foo")).matches("/foo/bar/%2e%2e"));
 
         Path path = new Path(URI.create("/%61/%2f/%63"));
         assertTrue(path.matches("/a/{slash}/{c}"));
@@ -73,4 +74,29 @@ public class PathTest {
         assertEquals("c", path.get("c"));
     }
 
+    @Test
+    public void testInvalidPaths() {
+        assertInvalid(URI.create("/foo/../bar"));
+        assertInvalid(URI.create("/foo/%2e%2e/bar"));
+        assertInvalidPathSpec(URI.create("/foo/bar"), "/foo/bar/..");
+        assertInvalidPathSpec(URI.create("/foo/bar"), "/foo/../bar");
+    }
+
+    private void assertInvalid(URI uri) {
+        try {
+            new Path(uri);
+            fail("Expected exception");
+        } catch (IllegalArgumentException ignored) {
+        }
+    }
+
+    private void assertInvalidPathSpec(URI uri, String pathSpec) {
+        try {
+            Path path = new Path(uri);
+            path.matches(pathSpec);
+            fail("Expected exception");
+        } catch (IllegalArgumentException ignored) {
+        }
+    }
+
 }