Merge pull request #32481 from elpaso/bugfix-gh-32475-server-security…

…-access-control Fix server WMS access control security issue
qgis · Oct 29, 2019 · a04354c · a04354c
2 parents 4eaab04 + 28d6822
commit a04354c
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 3 deletions.
diff --git a/src/server/services/wms/qgswmsrenderer.cpp b/src/server/services/wms/qgswmsrenderer.cpp
@@ -3073,7 +3073,7 @@ namespace QgsWms
     {
       const QgsWmsParametersLayer param = mContext.parameters( *layer );
 
-      if ( param.mNickname.isEmpty() )
+      if ( ! mContext.layersToRender().contains( layer ) )
       {
         continue;
       }

diff --git a/tests/src/python/test_qgsserver_accesscontrol_wms.py b/tests/src/python/test_qgsserver_accesscontrol_wms.py
@@ -12,11 +12,22 @@
 
 print('CTEST_FULL_OUTPUT')
 
+import os
+import json
 from qgis.testing import unittest
 import urllib.request
 import urllib.parse
 import urllib.error
 from test_qgsserver_accesscontrol import TestQgsServerAccessControl
+from utilities import unitTestDataPath
+
+from qgis.core import QgsProject
+from qgis.server import (
+    QgsServer,
+    QgsBufferServerRequest,
+    QgsBufferServerResponse,
+    QgsAccessControlFilter
+)
 
 
 class TestQgsServerAccessControlWMS(TestQgsServerAccessControl):
@@ -97,8 +108,8 @@ def test_wms_getprojectsettings(self):
             str(response).find("name=\"Country\"") != -1,
             "No Country layer in GetProjectSettings\n%s" % response)
         self.assertTrue(
-            str(response).find("name=\"Country\"")
-            < str(response).find("name=\"Hello\""),
+            str(response).find("name=\"Country\"") <
+            str(response).find("name=\"Hello\""),
             "Hello layer not after Country layer\n%s" % response)
 
         response, headers = self._get_restricted(query_string)
@@ -898,6 +909,42 @@ def test_wms_getfeatureinfo_projectsubsetstring2(self):
             str(response).find("<qgs:pk>") != -1,
             "Unexpected result from GetFeatureInfo Hello/2\n%s" % response)
 
+    def test_security_issue_gh32475(self):
+        """Test access control security issue GH 32475"""
+
+        class Filter(QgsAccessControlFilter):
+            def layerFilterSubsetString(self, layer):
+                handler = iface.requestHandler()
+                if handler.parameter("LAYER_PERM") == "yes":
+                    if layer.name() == "as_symbols" or layer.shortName() == "as_symbols":
+                        return "\"gid\" != 1"
+                return None
+
+        def _gfi(restrict, layers):
+            qs = ("?SERVICE=WMS&VERSION=1.3.0&REQUEST=GetFeatureInfo&"
+                  + "BBOX=612616,5810132,619259,5813237"
+                  + "&CRS=EPSG:25832&WIDTH=2759&HEIGHT=1290&&STYLES="
+                  + "&FORMAT=application/json&QUERY_LAYERS=%s"
+                  + "&INFO_FORMAT=application/json&I=508&J=560&FEATURE_COUNT=10") % layers
+            if restrict:
+                qs = qs + "&LAYER_PERM=yes"
+            request = QgsBufferServerRequest(qs)
+            response = QgsBufferServerResponse()
+            server.handleRequest(request, response, project)
+            return json.loads(bytes(response.body()).decode('utf8'))['features']
+
+        server = self._server
+        project = QgsProject()
+        project.read(os.path.join(unitTestDataPath('qgis_server'), 'test_project_wms_grouped_nested_layers.qgs'))
+        iface = server.serverInterface()
+        filter = Filter(iface)
+        iface.registerAccessControl(filter, 100)
+
+        self.assertEqual(len(_gfi(False, 'areas and symbols')), 1)
+        self.assertEqual(len(_gfi(True, 'areas and symbols')), 0)
+        self.assertEqual(len(_gfi(False, 'as_symbols')), 1)
+        self.assertEqual(len(_gfi(True, 'as_symbols')), 0)
+
 
 if __name__ == "__main__":
     unittest.main()