[GWC-1210] Improve input validation in ByteStreamController (#1211)

GeoWebCache · Feb 2, 2024 · c7f76bd · c7f76bd
1 parent 9b55d9a
commit c7f76bd
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 14 deletions.
diff --git a/geowebcache/rest/src/main/java/org/geowebcache/rest/controller/ByteStreamController.java b/geowebcache/rest/src/main/java/org/geowebcache/rest/controller/ByteStreamController.java
@@ -15,9 +15,9 @@
  */
 package org.geowebcache.rest.controller;
 
+import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
-import java.io.UnsupportedEncodingException;
 import java.net.URL;
 import java.net.URLDecoder;
 import java.util.List;
@@ -79,22 +79,21 @@ protected URL getResource(String path) {
 
     // "gwc/rest/web/openlayers3/ol.js" -> openlayers3/ol.js
     // "/rest/web/openlayers3/ol.js" -> openlayers3/ol.js
-    String getFileName(HttpServletRequest request) {
-        String path = request.getPathInfo();
-        if (path.indexOf("/rest/web") != 0) {
-            path = path.substring(path.indexOf("/rest/web"));
-        }
-        return path.substring("/rest/web/".length());
+    String getFileName(HttpServletRequest request) throws IOException {
+        String path =
+                URLDecoder.decode(request.getRequestURI(), "UTF-8")
+                        .substring(request.getContextPath().length())
+                        .replace(File.separatorChar, '/');
+        int index = path.indexOf("/rest/web/");
+        return index < 0 ? null : path.substring(index + "/rest/web/".length());
     }
 
     @RequestMapping(value = "/web/**", method = RequestMethod.GET)
-    ResponseEntity<?> doGet(HttpServletRequest request, HttpServletResponse response) {
-        final String filename;
-        try {
-            filename = URLDecoder.decode(getFileName(request), "UTF-8");
-        } catch (UnsupportedEncodingException e1) {
-            throw new IllegalStateException(
-                    "Could not decode encoding UTF-8", e1); // Should never happen
+    ResponseEntity<?> doGet(HttpServletRequest request, HttpServletResponse response)
+            throws IOException {
+        final String filename = getFileName(request);
+        if (filename == null || filename.isEmpty()) {
+            return new ResponseEntity<>(HttpStatus.NOT_FOUND);
         }
 
         // Just to make sure we don't allow access to arbitrary resources

diff --git a/geowebcache/rest/src/test/java/org/geowebcache/rest/controller/ByteStreamControllerTest.java b/geowebcache/rest/src/test/java/org/geowebcache/rest/controller/ByteStreamControllerTest.java
@@ -15,10 +15,12 @@
 package org.geowebcache.rest.controller;
 
 import static org.hamcrest.Matchers.startsWith;
+import static org.junit.Assume.assumeTrue;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 
+import org.apache.commons.lang3.SystemUtils;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -100,4 +102,18 @@ public void testBackreference2() throws Exception {
         mockMvc.perform(get("/rest/web/foo/../../../shouldnt/access/test.png"))
                 .andExpect(status().is4xxClientError());
     }
+
+    @Test
+    public void testBackreferenceWindows() throws Exception {
+        assumeTrue(SystemUtils.IS_OS_WINDOWS);
+        mockMvc.perform(get("/rest/web/..\\..\\shouldnt/access/test.png"))
+                .andExpect(status().is4xxClientError());
+    }
+
+    @Test
+    public void testBackreferenceWindows2() throws Exception {
+        assumeTrue(SystemUtils.IS_OS_WINDOWS);
+        mockMvc.perform(get("/rest/web/foo\\..\\..\\..\\shouldnt/access/test.png"))
+                .andExpect(status().is4xxClientError());
+    }
 }