fix: prevent passing bad character to wp (#11099) (#11116)

The webpack dev-server does not escape " character, as it is not valid URL. This limitation was not checked when passing request to it via DevModeHandlerImpl.
vaadin · May 31, 2021 · ca12a54 · ca12a54
1 parent 2a25583
commit ca12a54
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 3 deletions.
diff --git a/flow-server/src/main/java/com/vaadin/flow/server/DevModeHandler.java b/flow-server/src/main/java/com/vaadin/flow/server/DevModeHandler.java
@@ -91,6 +91,10 @@ public final class DevModeHandler implements RequestHandler {
 
     private static final AtomicReference<DevModeHandler> atomicHandler = new AtomicReference<>();
 
+    // webpack dev-server allows " character if passed through, need to
+    // explicitly check requests for it
+    private static final Pattern WEBPACK_ILLEGAL_CHAR_PATTERN = Pattern
+            .compile("\"|%22");
     // It's not possible to know whether webpack is ready unless reading output
     // messages. When webpack finishes, it writes either a `Compiled` or a
     // `Failed` in the last line
@@ -321,8 +325,10 @@ public boolean serveDevModeRequest(HttpServletRequest request,
         // a valid request for webpack-dev-server should start with '/VAADIN/'
         String requestFilename = request.getPathInfo();
 
-        if (HandlerHelper.isPathUnsafe(requestFilename)) {
-            getLogger().info(HandlerHelper.UNSAFE_PATH_ERROR_MESSAGE_PATTERN,
+        if (HandlerHelper.isPathUnsafe(requestFilename)
+                || WEBPACK_ILLEGAL_CHAR_PATTERN.matcher(requestFilename)
+                        .find()) {
+            getLogger().info("Blocked attempt to access file: {}",
                     requestFilename);
             response.setStatus(HttpServletResponse.SC_FORBIDDEN);
             return true;
@@ -408,6 +414,7 @@ private boolean checkWebpackConnection() {
      */
     public HttpURLConnection prepareConnection(String path, String method)
             throws IOException {
+        // path should have been checked at this point for any outside requests
         URL uri = new URL(WEBPACK_HOST + ":" + getPort() + path);
         HttpURLConnection connection = (HttpURLConnection) uri.openConnection();
         connection.setRequestMethod(method);

diff --git a/flow-server/src/main/java/com/vaadin/flow/server/HandlerHelper.java b/flow-server/src/main/java/com/vaadin/flow/server/HandlerHelper.java
@@ -16,6 +16,7 @@
 
 package com.vaadin.flow.server;
 
+import javax.servlet.http.HttpServletRequest;
 import java.io.Serializable;
 import java.io.UnsupportedEncodingException;
 import java.net.URLDecoder;

diff --git a/flow-server/src/test/java/com/vaadin/flow/server/DevModeHandlerTest.java b/flow-server/src/test/java/com/vaadin/flow/server/DevModeHandlerTest.java
@@ -21,12 +21,12 @@
 import javax.servlet.ServletOutputStream;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
-
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.IOException;
 import java.lang.reflect.Field;
 import java.net.ConnectException;
+import java.net.HttpURLConnection;
 import java.net.InetSocketAddress;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
@@ -546,6 +546,46 @@ public void start_serverPortDoesNotWork_throws() throws Exception {
         handler.join();
     }
 
+    @Test
+    public void serveDevModeRequest_uriForDevmodeGizmo_goesToWebpack()
+            throws Exception {
+        HttpServletRequest request = prepareRequest(
+                "/VAADIN/build/vaadin-devmodeGizmo-f679dbf313191ec3d018.cache.js");
+        HttpServletResponse response = prepareResponse();
+
+        final String manifestJsonResponse = "{ \"sw.js\": "
+                + "\"sw.js\", \"index.html\": \"index.html\" }";
+        int port = prepareHttpServer(0, HTTP_OK, manifestJsonResponse);
+
+        DevModeHandler devModeHandler = DevModeHandler.start(port,
+                configuration, npmFolder,
+                CompletableFuture.completedFuture(null));
+        devModeHandler.join();
+
+        assertTrue(devModeHandler.serveDevModeRequest(request, response));
+        assertEquals(HTTP_OK, responseStatus);
+    }
+
+    @Test
+    public void serveDevModeRequest_uriWithScriptInjected_returnsImmediatelyAndSetsForbiddenStatus()
+            throws Exception {
+        HttpServletRequest request = prepareRequest(
+                "/VAADIN/build/vaadin-devmodeGizmo-f679dbf313191ec3d018.cache%3f%22onload=%22alert(1)");
+        HttpServletResponse response = prepareResponse();
+
+        final String manifestJsonResponse = "{ \"sw.js\": "
+                + "\"sw.js\", \"index.html\": \"index.html\" }";
+        int port = prepareHttpServer(0, HTTP_OK, manifestJsonResponse);
+
+        DevModeHandler devModeHandler = DevModeHandler.start(port,
+                configuration, npmFolder,
+                CompletableFuture.completedFuture(null));
+        devModeHandler.join();
+
+        assertTrue(devModeHandler.serveDevModeRequest(request, response));
+        assertEquals(HTTP_FORBIDDEN, responseStatus);
+    }
+
     @Test
     public void serveDevModeRequest_uriWithDirectoryChangeWithSlash_returnsImmediatelyAndSetsForbiddenStatus()
             throws IOException {