Safe implementation of static resources

jknack · jknack · commit 182d1f194cdf · 2020-05-03T22:03:42.000-03:00
diff --git a/jooby/src/main/java/io/jooby/AssetSource.java b/jooby/src/main/java/io/jooby/AssetSource.java
@@ -43,7 +43,8 @@ public interface AssetSource {
    * (including jar files).
    *
    * @param loader Class loader.
-   * @param location Classpath location.
+   * @param location Classpath location. For security reasons root of classpath <code>/</code> is
+   *     disallowed.
    * @return An asset source.
    */
   static @Nonnull AssetSource create(@Nonnull ClassLoader loader, @Nonnull String location) {
diff --git a/jooby/src/main/java/io/jooby/Router.java b/jooby/src/main/java/io/jooby/Router.java
@@ -612,13 +612,13 @@ interface Match {
   }
 
   /**
-   * Add a static resource handler. Static resources are resolved from root classpath.
+   * Add a static resource handler. Static resources are resolved from `/static` class path folder.
    *
    * @param pattern Path pattern.
    * @return A route.
    */
   default @Nonnull Route assets(@Nonnull String pattern) {
-    return assets(pattern, AssetSource.create(getClass().getClassLoader(), "/"));
+    return assets(pattern, AssetSource.create(getClass().getClassLoader(), "/static"));
   }
 
   /**
diff --git a/jooby/src/main/java/io/jooby/internal/ClassPathAssetSource.java b/jooby/src/main/java/io/jooby/internal/ClassPathAssetSource.java
@@ -29,6 +29,10 @@ public class ClassPathAssetSource implements AssetSource {
   private final String prefix;
 
   public ClassPathAssetSource(ClassLoader loader, String source) {
+    if (source == null || source.trim().length() == 0 || source.trim().equals("/")) {
+      throw new IllegalArgumentException(
+          "For security reasons root classpath access is not allowed: " + source);
+    }
     this.loader = loader;
     this.source = source.startsWith("/") ? source.substring(1) : source;
     this.prefix = sourcePrefix(this.source);
diff --git a/jooby/src/test/java/io/jooby/internal/ClassPathAssetSourceTest.java b/jooby/src/test/java/io/jooby/internal/ClassPathAssetSourceTest.java
@@ -14,9 +14,24 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 public class ClassPathAssetSourceTest {
 
+  @Test
+  public void disallowedAccessToRootClasspath() {
+    assertThrows(IllegalArgumentException.class,
+        () -> new ClassPathAssetSource(getClass().getClassLoader(), null));
+    assertThrows(IllegalArgumentException.class,
+        () -> new ClassPathAssetSource(getClass().getClassLoader(), ""));
+    assertThrows(IllegalArgumentException.class,
+        () -> new ClassPathAssetSource(getClass().getClassLoader(), "  "));
+    assertThrows(IllegalArgumentException.class,
+        () -> new ClassPathAssetSource(getClass().getClassLoader(), "/"));
+    assertThrows(IllegalArgumentException.class,
+        () -> new ClassPathAssetSource(getClass().getClassLoader(), " / "));
+  }
+
   @Test
   public void checkclasspathFiles() {
     assetSource("/META-INF/resources/webjars/vue/" + VUE, source -> {
@@ -47,7 +62,7 @@ public void checkclasspathFiles() {
       assertEquals(MediaType.js, vuejs.getContentType());
     });
 
-    assetSource("/", source -> {
+    assetSource("/log", source -> {
       Asset logback = source.resolve("logback.xml");
       assertNotNull(logback);
       assertEquals(MediaType.xml, logback.getContentType());
diff --git a/jooby/src/test/resources/log/logback.xml b/jooby/src/test/resources/log/logback.xml
@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<configuration>
+  <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+    <encoder>
+      <pattern>%-5p [%d{ISO8601}] [%thread] %msg%n</pattern>
+    </encoder>
+  </appender>
+
+  <root level="INFO">
+    <appender-ref ref="STDOUT"/>
+  </root>
+</configuration>

Original file line number	Diff line number	Diff line change
`@@ -612,13 +612,13 @@ interface Match {`
`612`	`612`	`}`
`613`	`613`
`614`	`614`	`/**`
`615`		`- * Add a static resource handler. Static resources are resolved from root classpath.`
	`615`	+ * Add a static resource handler. Static resources are resolved from `/static` class path folder.
`616`	`616`	`*`
`617`	`617`	`* @param pattern Path pattern.`
`618`	`618`	`* @return A route.`
`619`	`619`	`*/`
`620`	`620`	`default @Nonnull Route assets(@Nonnull String pattern) {`
`621`		`- return assets(pattern, AssetSource.create(getClass().getClassLoader(), "/"));`
	`621`	`+ return assets(pattern, AssetSource.create(getClass().getClassLoader(), "/static"));`
`622`	`622`	`}`
`623`	`623`
`624`	`624`	`/**`