Allow serving static files in subdirectories

Change-Id: I82447dd9ce47ffc44727d9f886a45a9fa1a2463e
Signed-Off-By: James Y Knight <jyknight@google.com>
(cherry picked from commit b8ddd05fbf98c63b70bfa3a714952b43bcad549a)

1 files changed, 29 insertions, 10 deletions

diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/StaticServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/StaticServlet.java
index ea587a4c..c33a2aa7 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/StaticServlet.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/StaticServlet.java
@@ -89,34 +89,53 @@ public class StaticServlet extends HttpServlet {
   }
 
   private final File staticBase;
+  private final String staticBasePath;
 
   @Inject
   StaticServlet(final SitePaths site) {
-    staticBase = site.static_dir;
+    File f;
+    try {
+      f = site.static_dir.getCanonicalFile();
+    } catch (IOException e) {
+      f = site.static_dir.getAbsoluteFile();
+    }
+    staticBase = f;
+    staticBasePath = staticBase.getPath() + File.separator;
   }
 
   private File local(final HttpServletRequest req) {
     final String name = req.getPathInfo();
-    if (name.length() < 2 || !name.startsWith("/")) {
+    if (name.length() < 2 || !name.startsWith("/") || isUnreasonableName(name)) {
       // Too short to be a valid file name, or doesn't start with
       // the path info separator like we expected.
       //
       return null;
     }
 
-    if (name.indexOf('/', 1) > 0 || name.indexOf('\\', 1) > 0) {
-      // Contains a path separator. Don't serve it as the client
-      // might be trying something evil like "/../../etc/passwd".
-      // This static servlet is just meant to facilitate simple
-      // assets like banner images.
-      //
-      return null;
+    final File p = new File(staticBase, name.substring(1));
+
+    // Ensure that the requested file is *actually* within the static dir base.
+    try {
+      if (!p.getCanonicalFile().getPath().startsWith(staticBasePath))
+        return null;
+    } catch (IOException e) {
+        return null;
     }
 
-    final File p = new File(staticBase, name.substring(1));
     return p.isFile() ? p : null;
   }
 
+  private static boolean isUnreasonableName(String name) {
+    if (name.charAt(name.length() -1) == '/') return true; // no suffix
+    if (name.indexOf('\\') >= 0) return true; // no windows/dos stlye paths
+    if (name.startsWith("../")) return true; // no "../etc/passwd"
+    if (name.contains("/../")) return true; // no "foo/../etc/passwd"
+    if (name.contains("/./")) return true; // "foo/./foo" is insane to ask
+    if (name.contains("//")) return true; // windows UNC path can be "//..."
+
+    return false; // is a reasonable name
+  }
+
   @Override
   protected long getLastModified(final HttpServletRequest req) {
     final File p = local(req);