Asset path traversal with client that doesn't resolve the ../

Author: jknack

jooby/src/main/java/io/jooby/internal/ClassPathAssetSource.java

@@ -14,6 +14,7 @@
 import java.net.URL;
 import java.net.URLConnection;
 import java.nio.file.Files;
+import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.jar.JarFile;
 import java.util.zip.ZipEntry;
@@ -40,7 +41,16 @@ public ClassPathAssetSource(ClassLoader loader, String source) {
   }
 
   @Nullable @Override public Asset resolve(@Nonnull String path) {
-    String fullpath = isDir ? prefix + path : source;
+    String fullpath;
+    if (isDir) {
+      fullpath = safePath(prefix + path);
+      if (!fullpath.startsWith(prefix)) {
+        return null;
+      }
+    } else {
+      fullpath = source;
+    }
+
     URL resource = loader.getResource(fullpath);
     if (resource == null) {
       return null;
@@ -85,4 +95,23 @@ private boolean isDirectory(ClassLoader loader, String base) {
       return true;
     }
   }
+
+  private static String safePath(String path) {
+    if (path.indexOf("./") > 0) {
+      return normalize(path.split("/"));
+    }
+    return path;
+  }
+
+  private static String normalize(String[] segments) {
+    Path path = Paths.get(segments[0]);
+    for (int i = 1; i < segments.length; i++) {
+      path = path.resolve(segments[i]);
+    }
+    StringBuilder buffer = new StringBuilder();
+    for (Path segment : path.normalize()) {
+      buffer.append("/").append(segment);
+    }
+    return buffer.substring(1);
+  }
 }

modules/jooby-bom/pom.xml

@@ -16,12 +16,12 @@
   <!-- THIS FILE IS AUTO GENERATED. DON'T EDIT -->
 
   <properties>
-  <jooby.version>2.8.2</jooby.version>
+  <jooby.version>2.8.3-SNAPSHOT</jooby.version>
   <HikariCP.version>3.4.4</HikariCP.version>
   <archetype-packaging.version>3.1.2</archetype-packaging.version>
   <asm.version>8.0.1</asm.version>
   <auto-service.version>1.0-rc6</auto-service.version>
-  <aws-java-sdk.version>1.11.770</aws-java-sdk.version>
+  <aws-java-sdk.version>1.11.778</aws-java-sdk.version>
   <boringssl.version>2.0.27.Final</boringssl.version>
   <bucket4j-core.version>4.10.0</bucket4j-core.version>
   <checkstyle.version>8.32</checkstyle.version>
@@ -54,9 +54,9 @@
   <jdbi.version>3.12.2</jdbi.version>
   <jetty.version>9.4.28.v20200408</jetty.version>
   <jfiglet.version>0.0.8</jfiglet.version>
-  <jmespath-java.version>1.11.779</jmespath-java.version>
-  <jooby-maven-plugin.version>2.8.2</jooby-maven-plugin.version>
-  <jooby.version>2.8.2</jooby.version>
+  <jmespath-java.version>1.11.778</jmespath-java.version>
+  <jooby-maven-plugin.version>2.8.3-SNAPSHOT</jooby-maven-plugin.version>
+  <jooby.version>2.8.3-SNAPSHOT</jooby.version>
   <json.version>20190722</json.version>
   <jsonwebtoken.version>0.11.1</jsonwebtoken.version>
   <jsr305.version>3.0.2</jsr305.version>
@@ -93,6 +93,7 @@
   <mysql-connector-java.version>8.0.20</mysql-connector-java.version>
   <netty.version>4.1.49.Final</netty.version>
   <nexus-staging-maven-plugin.version>1.6.8</nexus-staging-maven-plugin.version>
+  <node.version>v12.16.3</node.version>
   <okhttp.version>4.6.0</okhttp.version>
   <pac4j.version>4.0.0</pac4j.version>
   <pebble.version>3.1.2</pebble.version>
@@ -107,7 +108,6 @@
   <spring.version>5.2.5.RELEASE</spring.version>
   <stork-maven-plugin.version>3.0.0</stork-maven-plugin.version>
   <swagger-parser.version>2.0.19</swagger-parser.version>
-  <swagger-ui.version>3.25.2</swagger-ui.version>
   <swagger.version>2.1.2</swagger.version>
   <thymeleaf.version>3.0.11.RELEASE</thymeleaf.version>
   <truth.version>1.0.1</truth.version>
@@ -551,12 +551,6 @@
       <version>${swagger-parser.version}</version>
       <type>jar</type>
     </dependency>
-    <dependency>
-      <groupId>org.webjars</groupId>
-      <artifactId>swagger-ui</artifactId>
-      <version>${swagger-ui.version}</version>
-      <type>jar</type>
-    </dependency>
     <dependency>
       <groupId>org.unbescape</groupId>
       <artifactId>unbescape</artifactId>
@@ -941,6 +935,12 @@
       <version>${plexus-utils.version}</version>
       <type>jar</type>
     </dependency>
+    <dependency>
+      <groupId>com.amazonaws</groupId>
+      <artifactId>aws-java-sdk-iotsitewise</artifactId>
+      <version>${aws-java-sdk.version}</version>
+      <type>jar</type>
+    </dependency>
     <dependency>
       <groupId>com.amazonaws</groupId>
       <artifactId>aws-java-sdk-synthetics</artifactId>

tests/pom.xml

@@ -154,6 +154,12 @@
       <artifactId>vue</artifactId>
       <version>2.6.11</version>
     </dependency>
+
+    <dependency>
+      <groupId>org.apache.httpcomponents</groupId>
+      <artifactId>fluent-hc</artifactId>
+      <version>4.5.12</version>
+    </dependency>
   </dependencies>
 
   <build>

tests/src/test/java/io/jooby/Issue1639.java

@@ -2,6 +2,7 @@
 
 import io.jooby.junit.ServerTest;
 import io.jooby.junit.ServerTestRunner;
+import org.apache.http.client.fluent.Request;
 
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -40,6 +41,9 @@ public void shouldNotAccessToClassFromCpAssetSource(ServerTestRunner runner) {
     runner.define(app -> {
       app.assets("/static/?*", "/static");
     }).ready(client -> {
+      client.get("/static/foo/../js/index.js", rsp -> {
+        assertEquals("(function () { console.log('index.js');});", rsp.body().string().trim());
+      });
       client.get("/static/js/index.js", rsp -> {
         assertEquals("(function () { console.log('index.js');});", rsp.body().string().trim());
       });
@@ -54,6 +58,39 @@ public void shouldNotAccessToClassFromCpAssetSource(ServerTestRunner runner) {
     });
   }
 
+  @ServerTest
+  public void shouldNotAccessToClassFromCpAssetSourceWithDifferentClient(ServerTestRunner runner) {
+    runner.define(app -> {
+      app.assets("/static/?*", "/static");
+    }).ready(client -> {
+      assertEquals(404,
+          Request
+              .Get("http://localhost:" + client.getPort() + "/static/../io/jooby/Issue1639.class")
+              .execute()
+              .returnResponse()
+              .getStatusLine().getStatusCode());
+
+      assertEquals("(function () { console.log('index.js');});",
+          Request.Get("http://localhost:" + client.getPort() + "/static/foo/../js/index.js")
+              .execute()
+              .returnContent()
+              .asString().trim());
+
+      assertEquals("(function () { console.log('index.js');});",
+          Request.Get("http://localhost:" + client.getPort() + "/static/js/index.js")
+              .execute()
+              .returnContent()
+              .asString().trim());
+
+      assertEquals(404,
+          Request.Get(
+              "http://localhost:" + client.getPort() + "/static/..%252fio/jooby/Issue1639.class")
+              .execute()
+              .returnResponse()
+              .getStatusLine().getStatusCode());
+    });
+  }
+
   private static Path userdir(String... segments) {
     Path path = Paths.get(System.getProperty("user.dir"));
     for (String segment : segments) {