Fix: Path traversal vulnerability in StaticFileHandler (#65)

* Prevent path traversal and sanitize URI in StaticFileHandler.

* Add test for path traversal protection and support 403 responses.

* Add tests for URI sanitization and handling of encoded/special URLs.

* Add test for null byte injection prevention in StaticFileHandler

* Improve StaticFileHandler path traversal handling and test coverage

* Improve URI sanitization and add test for 404 response handling in StaticFileHandler

Author: apaegs

src/main/java/org/example/StaticFileHandler.java

@@ -23,39 +23,45 @@ public StaticFileHandler(String webRoot) {
     }
 
     private void handleGetRequest(String uri) throws IOException {
-        // Security: Prevent path traversal attacks (e.g. GET /../../etc/passwd)
+        // Sanitize URI
+        int q = uri.indexOf('?');
+        if (q >= 0) uri = uri.substring(0, q);
+        int h = uri.indexOf('#');
+        if (h >= 0) uri = uri.substring(0, h);
+        uri = uri.replace("\0", "");
+        if (uri.startsWith("/")) uri = uri.substring(1);
+
+        // Path traversal check
         File root = new File(WEB_ROOT).getCanonicalFile();
         File file = new File(root, uri).getCanonicalFile();
-
         if (!file.toPath().startsWith(root.toPath())) {
-            fileBytes = "403 Forbidden".getBytes();
+            fileBytes = "403 Forbidden".getBytes(java.nio.charset.StandardCharsets.UTF_8);
             statusCode = 403;
             return;
         }
 
-        if (file.exists()) {
+        // Read file
+        if (file.isFile()) {
             fileBytes = Files.readAllBytes(file.toPath());
             statusCode = 200;
         } else {
             File errorFile = new File(WEB_ROOT, "pageNotFound.html");
-            if (errorFile.exists()) {
+            if (errorFile.isFile()) {
                 fileBytes = Files.readAllBytes(errorFile.toPath());
             } else {
-                fileBytes = "404 Not Found".getBytes();
+                fileBytes = "404 Not Found".getBytes(java.nio.charset.StandardCharsets.UTF_8);
             }
             statusCode = 404;
         }
     }
 
     public void sendGetRequest(OutputStream outputStream, String uri) throws IOException {
         handleGetRequest(uri);
-
         HttpResponseBuilder response = new HttpResponseBuilder();
         response.setStatusCode(statusCode);
         // Use MimeTypeDetector instead of hardcoded text/html
         response.setContentTypeFromFilename(uri);
         response.setBody(fileBytes);
-
         outputStream.write(response.build());
         outputStream.flush();
     }

src/main/java/org/example/http/HttpResponseBuilder.java

@@ -36,14 +36,15 @@ public class HttpResponseBuilder {
     public void setStatusCode(int statusCode) {
         this.statusCode = statusCode;
     }
+
     public void setBody(String body) {
         this.body = body != null ? body : "";
-        this.bytebody = null; // Clear byte body when setting string body
+        this.bytebody = null;
     }
 
     public void setBody(byte[] body) {
         this.bytebody = body;
-        this.body = ""; // Clear string body when setting byte body
+        this.body = "";
     }
 
     public void setHeaders(Map<String, String> headers) {
@@ -65,7 +66,6 @@ public void setContentTypeFromFilename(String filename) {
      * @return Complete HTTP response (headers + body) as byte[]
      */
     public byte[] build() {
-        // Determine content body and length
         byte[] contentBody;
         int contentLength;
 
@@ -77,37 +77,29 @@ public byte[] build() {
             contentLength = contentBody.length;
         }
 
-        // Build headers as String
         StringBuilder headerBuilder = new StringBuilder();
 
-        // Status line
         String reason = REASON_PHRASES.getOrDefault(statusCode, "");
         headerBuilder.append(PROTOCOL).append(" ").append(statusCode);
         if (!reason.isEmpty()) {
             headerBuilder.append(" ").append(reason);
         }
         headerBuilder.append(CRLF);
 
-        // User-defined headers
         headers.forEach((k, v) -> headerBuilder.append(k).append(": ").append(v).append(CRLF));
 
-        // Auto-append Content-Length if not set.
         if (!headers.containsKey("Content-Length")) {
             headerBuilder.append("Content-Length: ").append(contentLength).append(CRLF);
         }
 
-        // Auto-append Connection if not set.
         if (!headers.containsKey("Connection")) {
             headerBuilder.append("Connection: close").append(CRLF);
         }
 
-        // Blank line before body
         headerBuilder.append(CRLF);
 
-        // Convert headers to bytes
         byte[] headerBytes = headerBuilder.toString().getBytes(StandardCharsets.UTF_8);
 
-        // Combine headers + body into single byte array
         byte[] response = new byte[headerBytes.length + contentBody.length];
         System.arraycopy(headerBytes, 0, response, 0, headerBytes.length);
         System.arraycopy(contentBody, 0, response, headerBytes.length, contentBody.length);

src/test/java/org/example/StaticFileHandlerTest.java

@@ -2,6 +2,8 @@
 
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
 
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
@@ -76,4 +78,60 @@ void test_file_that_does_not_exists_should_return_404() throws IOException {
 
     }
 
+    @Test
+    void test_path_traversal_should_return_403() throws IOException {
+        // Arrange
+        Path secret = tempDir.resolve("secret.txt");
+        Files.writeString(secret,"TOP SECRET");
+        Path webRoot = tempDir.resolve("www");
+        Files.createDirectories(webRoot);
+        StaticFileHandler handler = new StaticFileHandler(webRoot.toString());
+        ByteArrayOutputStream fakeOutput = new ByteArrayOutputStream();
+
+        // Act
+        handler.sendGetRequest(fakeOutput, "../secret.txt");
+
+        // Assert
+        String response = fakeOutput.toString();
+        assertFalse(response.contains("TOP SECRET"));
+        assertTrue(response.contains("HTTP/1.1 403 Forbidden"));
+    }
+
+    @ParameterizedTest
+    @CsvSource({
+            "index.html?foo=bar",
+            "index.html#section",
+            "/index.html"
+    })
+    void sanitized_uris_should_return_200(String uri) throws IOException {
+        // Arrange
+        Path webRoot = tempDir.resolve("www");
+        Files.createDirectories(webRoot);
+        Files.writeString(webRoot.resolve("index.html"), "Hello");
+        StaticFileHandler handler = new StaticFileHandler(webRoot.toString());
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+
+        // Act
+        handler.sendGetRequest(out, uri);
+
+        // Assert
+        assertTrue(out.toString().contains("HTTP/1.1 200 OK"));
+    }
+
+    @Test
+    void null_byte_injection_should_not_return_200() throws IOException {
+        // Arrange
+        Path webRoot = tempDir.resolve("www");
+        Files.createDirectories(webRoot);
+        StaticFileHandler handler = new StaticFileHandler(webRoot.toString());
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+
+        // Act
+        handler.sendGetRequest(out, "index.html\0../../etc/passwd");
+
+        // Assert
+        String response = out.toString();
+        assertFalse(response.contains("HTTP/1.1 200 OK"));
+        assertTrue(response.contains("HTTP/1.1 404 Not Found"));
+    }
 }