From 95a2e96d1f54e197f079b56a476d336472f6c487 Mon Sep 17 00:00:00 2001
From: Steve Ikeoka <steve.ikeoka@gdit.com>
Date: Thu, 1 Feb 2024 12:15:40 -0800
Subject: [PATCH] [GWC-1210] Improve input validation in ByteStreamController

---
 .../rest/controller/ByteStreamController.java | 27 +++++++++----------
 .../controller/ByteStreamControllerTest.java  | 16 +++++++++++
 2 files changed, 29 insertions(+), 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
index 38e471793..fa23daf7a 100644
--- 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
index 5e71cd2ae..5dc52a4b8 100644
--- 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());
+    }
 }