[GEOS-11153] Improve handling special characters in the WMS OpenLayer…

…s Format
geoserver · Oct 10, 2023 · 9a3365a · 9a3365a
1 parent d9568d7
commit 9a3365a
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 33 deletions.
diff --git a/src/wms/src/main/java/org/geoserver/wms/map/AbstractOpenLayersMapOutputFormat.java b/src/wms/src/main/java/org/geoserver/wms/map/AbstractOpenLayersMapOutputFormat.java
@@ -25,6 +25,7 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
+import org.apache.commons.text.StringEscapeUtils;
 import org.geoserver.catalog.Catalog;
 import org.geoserver.catalog.LayerGroupInfo;
 import org.geoserver.catalog.impl.LayerGroupStyle;
@@ -292,7 +293,7 @@ private List<String> styleNames(WMSMapContent mapContent) {
             MapLayerInfo info = mapContent.getRequest().getLayers().get(0);
             result = info.getOtherStyleNames();
         }
-        return result;
+        return result.stream().map(StringEscapeUtils::escapeHtml4).collect(Collectors.toList());
     }
 
     private List<String> getGroupStyleOrEmpty(WMSMapContent mapContent) {

diff --git a/src/wms/src/main/resources/org/geoserver/wms/map/OpenLayers2MapTemplate.ftl b/src/wms/src/main/resources/org/geoserver/wms/map/OpenLayers2MapTemplate.ftl
@@ -149,7 +149,7 @@
             
                 // setup tiled layer
                 tiled = new OpenLayers.Layer.WMS(
-                    "${layerName} - Tiled", "${baseUrl}/${servicePath}",
+                    "${layerName?js_string} - Tiled", "${baseUrl}/${servicePath?js_string}",
                     {
                         <#list parameters as param>
                         "${param.name?js_string}": '${param.value?js_string}',
@@ -168,7 +168,7 @@
                   
                 // setup single tiled layer
                 untiled = new OpenLayers.Layer.WMS(
-                    "${layerName} - Untiled", "${baseUrl}/${servicePath}",
+                    "${layerName?js_string} - Untiled", "${baseUrl}/${servicePath?js_string}",
                     {
                         <#list parameters as param>
                         "${param.name?js_string}": '${param.value?js_string}',
@@ -242,7 +242,7 @@
                     if(map.layers[0].params.FEATUREID) {
                         params.featureid = map.layers[0].params.FEATUREID;
                     }
-                    OpenLayers.loadURL("${baseUrl}/${servicePath}", params, this, setHTML, setHTML);
+                    OpenLayers.loadURL("${baseUrl}/${servicePath?js_string}", params, this, setHTML, setHTML);
                     OpenLayers.Event.stop(e);
                 });
             }

diff --git a/src/wms/src/main/resources/org/geoserver/wms/map/OpenLayers3MapTemplate.ftl b/src/wms/src/main/resources/org/geoserver/wms/map/OpenLayers3MapTemplate.ftl
@@ -257,7 +257,7 @@
       var untiled = new ol.layer.Image({
         source: new ol.source.ImageWMS({
           ratio: 1,
-          url: '${baseUrl}/${servicePath}',
+          url: '${baseUrl}/${servicePath?js_string}',
           params: {'FORMAT': format,
                    'VERSION': '1.1.1',  
              <#list parameters as param>
@@ -269,7 +269,7 @@
       var tiled = new ol.layer.Tile({
         visible: false,
         source: new ol.source.TileWMS({
-          url: '${baseUrl}/${servicePath}',
+          url: '${baseUrl}/${servicePath?js_string}',
           params: {'FORMAT': format, 
                    'VERSION': '1.1.1',
                    tiled: true,

diff --git a/src/wms/src/test/java/org/geoserver/wms/map/OpenLayersMapOutputFormatTest.java b/src/wms/src/test/java/org/geoserver/wms/map/OpenLayersMapOutputFormatTest.java
@@ -130,22 +130,28 @@ public void testXssFix() throws Exception {
         StyleInfo styleByName = catalog.getStyleByName("Default");
         Style basicStyle = styleByName.getStyle();
         FeatureLayer layer = new FeatureLayer(fs, basicStyle);
-        layer.setTitle("Title");
+        layer.setTitle("Title</foo");
         map.addLayer(layer);
         request.setFormat("application/openlayers");
-        String htmlDoc = getAsHTML(map);
-        // check that weird param is correctly encoded to avoid js code execution
-        int index =
-                htmlDoc.replace("\\n", "")
-                        .replace("\\r", "")
-                        .indexOf(
-                                "\"<\\/script><script>alert(\\'x-scripted\\');<\\/script><script>\": 'foo'");
-        assertTrue(index > -1);
-        index =
-                htmlDoc.replace("\\n", "")
-                        .replace("\\r", "")
-                        .indexOf("\"25064;ALERT(1)//419\": '1'");
-        assertTrue(index > -1);
+
+        StyleInfo otherStyle = new StyleInfoImpl(null);
+        otherStyle.setName("style<>");
+        try {
+            request.getLayers().get(0).getLayerInfo().getStyles().add(otherStyle);
+            String htmlDoc = getAsHTML(map);
+            // check that weird param is correctly encoded to avoid js code execution
+            assertThat(
+                    htmlDoc,
+                    containsString(
+                            "\"<\\/script><script>alert(\\'x-scripted\\');<\\/script><script>\": 'foo'"));
+            assertThat(htmlDoc, containsString("\"25064;ALERT(1)//419\": '1'"));
+            assertThat(htmlDoc, not(containsString(layer.getTitle())));
+            assertThat(htmlDoc, containsString("Title<\\/foo"));
+            assertThat(htmlDoc, not(containsString(otherStyle.getName())));
+            assertThat(htmlDoc, containsString("style&lt;&gt;"));
+        } finally {
+            request.getLayers().get(0).getLayerInfo().getStyles().remove(otherStyle);
+        }
     }
 
     @Test
@@ -472,19 +478,23 @@ public void testXssOL3() throws Exception {
         layer.setTitle("Title");
         map.addLayer(layer);
         request.setFormat("application/openlayers3");
-        String htmlDoc = getAsHTMLOL3(map);
-        // check that weird param is correctly encoded to avoid js code execution
-        int index =
-                htmlDoc.replace("\\n", "")
-                        .replace("\\r", "")
-                        .indexOf(
-                                "\"<\\/script><script>alert(\\'x-scripted\\');<\\/script><script>\": 'foo'");
-        assertTrue(index > -1);
-        index =
-                htmlDoc.replace("\\n", "")
-                        .replace("\\r", "")
-                        .indexOf("\"25064;ALERT(1)//419\": '1'");
-        assertTrue(index > -1);
+
+        StyleInfo otherStyle = new StyleInfoImpl(null);
+        otherStyle.setName("style<>");
+        try {
+            request.getLayers().get(0).getLayerInfo().getStyles().add(otherStyle);
+            String htmlDoc = getAsHTMLOL3(map);
+            // check that weird param is correctly encoded to avoid js code execution
+            assertThat(
+                    htmlDoc,
+                    containsString(
+                            "\"<\\/script><script>alert(\\'x-scripted\\');<\\/script><script>\": 'foo'"));
+            assertThat(htmlDoc, containsString("\"25064;ALERT(1)//419\": '1'"));
+            assertThat(htmlDoc, not(containsString(otherStyle.getName())));
+            assertThat(htmlDoc, containsString("style&lt;&gt;"));
+        } finally {
+            request.getLayers().get(0).getLayerInfo().getStyles().remove(otherStyle);
+        }
     }
 
     @Test