Permalink
Browse files

Merge pull request #701 from perwendel/fix-for-directory-traversal-is…

…sue-700

Fix for #700 - Arbitrary File Read Vulnerability
  • Loading branch information...
2 parents c57e572 + afeee2b commit efcb46c710e3f56805b9257a63d1306882f4faf9 @perwendel committed on GitHub Nov 6, 2016
@@ -22,6 +22,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import spark.staticfiles.DirectoryTraversal;
import spark.utils.Assert;
/**
@@ -78,7 +79,15 @@ protected AbstractFileResolvingResource getResource(String path) throws Malforme
}
}
- return (resource != null && resource.exists()) ? resource : null;
+ if (resource != null && resource.exists()) {
+ DirectoryTraversal.protectAgainstInClassPath(resource.getPath());
+ return resource;
+ } else {
+ return null;
+ }
+
+ } catch (DirectoryTraversal.DirectoryTraversalDetection directoryTraversalDetection) {
+ throw directoryTraversalDetection;
} catch (Exception e) {
if (LOG.isDebugEnabled()) {
LOG.debug(e.getClass().getSimpleName() + " when trying to get resource. " + e.getMessage());
@@ -22,6 +22,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import spark.staticfiles.DirectoryTraversal;
import spark.utils.Assert;
/**
@@ -77,7 +78,15 @@ protected AbstractFileResolvingResource getResource(String path) throws Malforme
}
}
- return (resource != null && resource.exists()) ? resource : null;
+ if (resource != null && resource.exists()) {
+ DirectoryTraversal.protectAgainstForExternal(resource.getPath());
+ return resource;
+ } else {
+ return null;
+ }
+
+ } catch (DirectoryTraversal.DirectoryTraversalDetection directoryTraversalDetection) {
+ throw directoryTraversalDetection;
} catch (Exception e) {
if (LOG.isDebugEnabled()) {
LOG.debug(e.getClass().getSimpleName() + " when trying to get resource. " + e.getMessage());
@@ -0,0 +1,30 @@
+package spark.staticfiles;
+
+import static spark.utils.StringUtils.removeLeadingAndTrailingSlashesFrom;
+
+/**
+ * Protecting against Directory traversal
+ */
+public class DirectoryTraversal {
+
+ public static void protectAgainstInClassPath(String path) {
+ if (!removeLeadingAndTrailingSlashesFrom(path).startsWith(StaticFilesFolder.local())) {
+ throw new DirectoryTraversalDetection("classpath");
+ }
+ }
+
+ public static void protectAgainstForExternal(String path) {
+ if (!removeLeadingAndTrailingSlashesFrom(path).startsWith(StaticFilesFolder.external())) {
+ throw new DirectoryTraversalDetection("external");
+ }
+ }
+
+ public static final class DirectoryTraversalDetection extends RuntimeException {
+
+ public DirectoryTraversalDetection(String msg) {
+ super(msg);
+ }
+
+ }
+
+}
@@ -69,15 +69,18 @@
*/
public boolean consume(HttpServletRequest httpRequest,
HttpServletResponse httpResponse) throws IOException {
+ try {
+ if (consumeWithFileResourceHandlers(httpRequest, httpResponse)) {
+ return true;
+ }
- if (consumeWithFileResourceHandlers(httpRequest, httpResponse)) {
- return true;
- }
-
- if (consumeWithJarResourceHandler(httpRequest, httpResponse)) {
- return true;
+ if (consumeWithJarResourceHandler(httpRequest, httpResponse)) {
+ return true;
+ }
+ } catch (DirectoryTraversal.DirectoryTraversalDetection directoryTraversalDetection) {
+ LOG.warn(directoryTraversalDetection.getMessage() + " directory traversal detection for path: "
+ + httpRequest.getPathInfo());
}
-
return false;
}
@@ -94,7 +97,7 @@ private boolean consumeWithFileResourceHandlers(HttpServletRequest httpRequest,
httpResponse.setHeader(MimeType.CONTENT_TYPE, MimeType.fromResource(resource));
customHeaders.forEach(httpResponse::setHeader); //add all user-defined headers to response
OutputStream wrappedOutputStream = GzipUtils.checkAndWrap(httpRequest, httpResponse, false);
-
+
IOUtils.copy(resource.getInputStream(), wrappedOutputStream);
wrappedOutputStream.flush();
wrappedOutputStream.close();
@@ -177,6 +180,8 @@ public synchronized void configure(String folder) {
} catch (IOException e) {
LOG.error("Error when creating StaticResourceHandler", e);
}
+
+ StaticFilesFolder.localConfiguredTo(folder);
staticResourcesSet = true;
}
@@ -197,7 +202,7 @@ private boolean configureJarCase(String folder, ClassPathResource resource) thro
staticResourcesSet = true;
return true;
}
-
+
LOG.error("Static file configuration failed.");
}
return false;
@@ -227,6 +232,8 @@ public synchronized void configureExternal(String folder) {
} catch (IOException e) {
LOG.error("Error when creating external StaticResourceHandler", e);
}
+
+ StaticFilesFolder.externalConfiguredTo(folder);
externalStaticResourcesSet = true;
}
@@ -0,0 +1,32 @@
+package spark.staticfiles;
+
+import static spark.utils.StringUtils.removeLeadingAndTrailingSlashesFrom;
+
+/**
+ * Created by Per Wendel on 2016-11-05.
+ */
+public class StaticFilesFolder {
+
+ private static volatile String local;
+ private static volatile String external;
+
+ public static final void localConfiguredTo(String folder) {
+
+ local = removeLeadingAndTrailingSlashesFrom(folder);
+ }
+
+ public static final void externalConfiguredTo(String folder) {
+
+ external = removeLeadingAndTrailingSlashesFrom(folder);
+ }
+
+ public static final String local() {
+ return local;
+ }
+
+ public static final String external() {
+ return external;
+ }
+
+
+}
@@ -390,4 +390,19 @@ public static String toString(byte[] bytes, String encoding) {
return str;
}
+ public static String removeLeadingAndTrailingSlashesFrom(String string) {
+
+ String trimmed = string;
+
+ if (trimmed.endsWith("/") || trimmed.endsWith("\\")) {
+ trimmed = trimmed.substring(0, trimmed.length() - 1);
+ }
+
+ if (trimmed.startsWith("/")) {
+ trimmed = trimmed.substring(1);
+ }
+
+ return trimmed;
+ }
+
}
@@ -19,6 +19,7 @@
import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
+import java.net.URLEncoder;
import org.junit.AfterClass;
import org.junit.Assert;
@@ -93,14 +94,14 @@ public static void setup() throws IOException {
@Test
public void testMimeTypes() throws Exception {
- Assert.assertEquals("text/html", doGet("/pages/index.html").headers.get("Content-Type"));
- Assert.assertEquals("application/javascript", doGet("/js/scripts.js").headers.get("Content-Type"));
- Assert.assertEquals("text/css", doGet("/css/style.css").headers.get("Content-Type"));
- Assert.assertEquals("image/png", doGet("/img/sparklogo.png").headers.get("Content-Type"));
- Assert.assertEquals("image/svg+xml", doGet("/img/sparklogo.svg").headers.get("Content-Type"));
+ Assert.assertEquals("text/html", doGet("/pages/index.html").headers.get("Content-Type"));
+ Assert.assertEquals("application/javascript", doGet("/js/scripts.js").headers.get("Content-Type"));
+ Assert.assertEquals("text/css", doGet("/css/style.css").headers.get("Content-Type"));
+ Assert.assertEquals("image/png", doGet("/img/sparklogo.png").headers.get("Content-Type"));
+ Assert.assertEquals("image/svg+xml", doGet("/img/sparklogo.svg").headers.get("Content-Type"));
Assert.assertEquals("application/octet-stream", doGet("/img/sparklogoPng").headers.get("Content-Type"));
Assert.assertEquals("application/octet-stream", doGet("/img/sparklogoSvg").headers.get("Content-Type"));
- Assert.assertEquals("text/html", doGet("/externalFile.html").headers.get("Content-Type"));
+ Assert.assertEquals("text/html", doGet("/externalFile.html").headers.get("Content-Type"));
}
@Test
@@ -137,10 +138,21 @@ public void testStaticFilePageHtml() throws Exception {
}
@Test
+ public void testDirectoryTraversalProtectionLocal() throws Exception {
+ String path = "/" + URLEncoder.encode("..\\spark\\") + "Spark.class";
+ SparkTestUtil.UrlResponse response = doGet(path);
+
+ Assert.assertEquals(404, response.status);
+ Assert.assertEquals(NOT_FOUND_BRO, response.body);
+
+ testGet();
+ }
+
+ @Test
public void testExternalStaticFile() throws Exception {
SparkTestUtil.UrlResponse response = doGet("/externalFile.html");
Assert.assertEquals(200, response.status);
- Assert.assertEquals("Content of external file", response.body);
+ Assert.assertEquals(CONTENT_OF_EXTERNAL_FILE, response.body);
testGet();
}
@@ -0,0 +1,139 @@
+/*
+ * Copyright 2015 - Per Wendel
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package spark;
+
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.net.URLEncoder;
+
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import spark.examples.exception.NotFoundException;
+import spark.util.SparkTestUtil;
+
+import static spark.Spark.exception;
+import static spark.Spark.externalStaticFileLocation;
+import static spark.Spark.get;
+
+/**
+ * Test external static files
+ */
+public class StaticFilesTestExternal {
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(StaticFilesTestExternal.class);
+
+ private static final String FO_SHIZZY = "Fo shizzy";
+ private static final String NOT_FOUND_BRO = "Not found bro";
+
+ private static final String EXTERNAL_FILE_NAME_HTML = "externalFile.html";
+
+ private static final String CONTENT_OF_EXTERNAL_FILE = "Content of external file";
+
+ private static SparkTestUtil testUtil;
+
+ private static File tmpExternalFile1;
+ private static File tmpExternalFile2;
+ private static File folderOutsideStaticFiles;
+
+ @AfterClass
+ public static void tearDown() {
+ Spark.stop();
+ if (tmpExternalFile1 != null) {
+ LOGGER.debug("tearDown(). Deleting tmp files");
+ tmpExternalFile1.delete();
+ tmpExternalFile2.delete();
+ folderOutsideStaticFiles.delete();
+ }
+ }
+
+ @BeforeClass
+ public static void setup() throws IOException {
+ testUtil = new SparkTestUtil(4567);
+
+ String directoryRoot = System.getProperty("java.io.tmpdir") + "sparkish";
+ new File(directoryRoot).mkdirs();
+
+ tmpExternalFile1 = new File(directoryRoot, EXTERNAL_FILE_NAME_HTML);
+
+ FileWriter writer = new FileWriter(tmpExternalFile1);
+ writer.write(CONTENT_OF_EXTERNAL_FILE);
+ writer.flush();
+ writer.close();
+
+ File root = new File(directoryRoot);
+
+ folderOutsideStaticFiles = new File(root.getAbsolutePath() + "/../dumpsterstuff");
+ folderOutsideStaticFiles.mkdirs();
+
+ String newFilePath = root.getAbsolutePath() + "/../dumpsterstuff/Spark.class";
+ tmpExternalFile2 = new File(newFilePath);
+ tmpExternalFile2.createNewFile();
+
+ externalStaticFileLocation(directoryRoot);
+
+ get("/hello", (q, a) -> FO_SHIZZY);
+
+ get("/*", (q, a) -> {
+ throw new NotFoundException();
+ });
+
+ exception(NotFoundException.class, (e, request, response) -> {
+ response.status(404);
+ response.body(NOT_FOUND_BRO);
+ });
+
+ Spark.awaitInitialization();
+ }
+
+ @Test
+ public void testExternalStaticFile() throws Exception {
+ SparkTestUtil.UrlResponse response = doGet("/externalFile.html");
+ Assert.assertEquals(200, response.status);
+ Assert.assertEquals(CONTENT_OF_EXTERNAL_FILE, response.body);
+
+ testGet();
+ }
+
+ @Test
+ public void testDirectoryTraversalProtectionExternal() throws Exception {
+ String path = "/" + URLEncoder.encode("..\\..\\spark\\") + "Spark.class";
+ SparkTestUtil.UrlResponse response = doGet(path);
+
+ Assert.assertEquals(404, response.status);
+ Assert.assertEquals(NOT_FOUND_BRO, response.body);
+
+ testGet();
+ }
+
+ private static void testGet() throws Exception {
+ SparkTestUtil.UrlResponse response = testUtil.doMethod("GET", "/hello", "");
+
+ Assert.assertEquals(200, response.status);
+ Assert.assertTrue(response.body.contains(FO_SHIZZY));
+ }
+
+ private SparkTestUtil.UrlResponse doGet(String fileName) throws Exception {
+ return testUtil.doMethod("GET", fileName, null);
+ }
+
+}
Oops, something went wrong.

0 comments on commit efcb46c

Please sign in to comment.