From b77dc8b806ff70d2dba2d8a799a0e1eab4790453 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Tue, 29 Oct 2019 11:43:02 +0100 Subject: [PATCH 1/2] Fix server WMS access control security issue Fixes #32475 --- src/server/services/wms/qgswmsrenderer.cpp | 2 +- .../test_qgsserver_accesscontrol_wms.py | 52 ++++++++++++++++++- 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/src/server/services/wms/qgswmsrenderer.cpp b/src/server/services/wms/qgswmsrenderer.cpp index a1dd5138b79..1faaba16045 100644 --- 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 index ae808591618..1782fc9f1cc 100644 --- a/tests/src/python/test_qgsserver_accesscontrol_wms.py +++ b/tests/src/python/test_qgsserver_accesscontrol_wms.py @@ -12,11 +12,22 @@ __copyright__ = 'Copyright 2015, The QGIS Project' 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 @@ class TestQgsServerAccessControlWMS(TestQgsServerAccessControl): 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) @@ -450,6 +461,7 @@ class TestQgsServerAccessControlWMS(TestQgsServerAccessControl): # # Subset String # # + def test_wms_getmap_subsetstring(self): query_string = "&".join(["%s=%s" % i for i in list({ "MAP": urllib.parse.quote(self.projectPath), @@ -898,6 +910,42 @@ class TestQgsServerAccessControlWMS(TestQgsServerAccessControl): str(response).find("") != -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() From 28d682243f4cd3ace05ed23e41d34d7703119b50 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Tue, 29 Oct 2019 15:38:33 +0100 Subject: [PATCH 2/2] Autopep8 --- tests/src/python/test_qgsserver_accesscontrol_wms.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/src/python/test_qgsserver_accesscontrol_wms.py b/tests/src/python/test_qgsserver_accesscontrol_wms.py index 1782fc9f1cc..37237b2e0e8 100644 --- a/tests/src/python/test_qgsserver_accesscontrol_wms.py +++ b/tests/src/python/test_qgsserver_accesscontrol_wms.py @@ -461,7 +461,6 @@ class TestQgsServerAccessControlWMS(TestQgsServerAccessControl): # # Subset String # # - def test_wms_getmap_subsetstring(self): query_string = "&".join(["%s=%s" % i for i in list({ "MAP": urllib.parse.quote(self.projectPath), @@ -924,9 +923,9 @@ class TestQgsServerAccessControlWMS(TestQgsServerAccessControl): 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 + + "&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)