From 94b341b240931c09989ce161fdfa11d099d14fd7 Mon Sep 17 00:00:00 2001 From: Jens Vagelpohl Date: Sat, 9 Sep 2023 10:48:10 +0200 Subject: [PATCH 1/2] - Tighten down the ZMI frame source logic to only allow site-local sources --- CHANGES.rst | 6 +++-- setup.py | 2 +- src/App/dtml/manage.dtml | 2 +- src/OFS/Application.py | 29 +++++++++++++++++++++++ src/OFS/tests/testApplication.py | 40 ++++++++++++++++++++++++++++++++ 5 files changed, 75 insertions(+), 4 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 1a0fc2cf19..1136f3d2e8 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -8,8 +8,10 @@ The change log for the previous version, Zope 4, is at https://github.com/zopefoundation/Zope/blob/4.x/CHANGES.rst -5.9 (unreleased) ----------------- +5.8.5 (unreleased) +------------------ + +- Tighten down the ZMI frame source logic to only allow site-local sources. - Update to newest compatible versions of dependencies. diff --git a/setup.py b/setup.py index c045010bb5..d80b72e718 100644 --- a/setup.py +++ b/setup.py @@ -29,7 +29,7 @@ def _read_file(filename): README = _read_file('README.rst') CHANGES = _read_file('CHANGES.rst') -version = '5.9.dev0' +version = '5.8.5.dev0' setup( name='Zope', diff --git a/src/App/dtml/manage.dtml b/src/App/dtml/manage.dtml index a342e71226..89800b76bc 100644 --- a/src/App/dtml/manage.dtml +++ b/src/App/dtml/manage.dtml @@ -19,7 +19,7 @@ name="manage_menu" marginwidth="2" marginheight="2" /> - " + " name="manage_main" marginwidth="2" marginheight="2" /> diff --git a/src/OFS/Application.py b/src/OFS/Application.py index 1a4ec252d0..5147894b7f 100644 --- a/src/OFS/Application.py +++ b/src/OFS/Application.py @@ -16,6 +16,7 @@ import os import sys from logging import getLogger +from urllib.parse import urlparse import Products import transaction @@ -105,6 +106,34 @@ def Redirect(self, destination, URL1): ZopeRedirect = Redirect + @security.protected(view_management_screens) + def getZMIMainFrameTarget(self, REQUEST): + """Utility method to get the right hand side ZMI frame source URL + + For cases where JavaScript is disabled the ZMI uses a simple REQUEST + variable ``came_from`` to set the source URL for the right hand side + ZMI frame. Since this value can be manipulated by the user it must be + sanity-checked first. + """ + parent_url = REQUEST['URL1'] + default = f'{parent_url}/manage_workspace' + came_from = REQUEST.get('came_from', None) + + if not came_from: + return default + + parsed_parent_url = urlparse(parent_url) + parsed_came_from = urlparse(came_from) + + # Only allow a passed-in ``came_from`` URL if it is local (just a path) + # or if the URL scheme and hostname are the same as our own + if (not parsed_came_from.scheme and not parsed_came_from.netloc) or \ + (parsed_parent_url.scheme == parsed_came_from.scheme and + parsed_parent_url.netloc == parsed_came_from.netloc): + return came_from + + return default + def __bobo_traverse__(self, REQUEST, name=None): if name is None: # Make this more explicit, otherwise getattr(self, name) diff --git a/src/OFS/tests/testApplication.py b/src/OFS/tests/testApplication.py index cc23fe6c5f..24a6005270 100644 --- a/src/OFS/tests/testApplication.py +++ b/src/OFS/tests/testApplication.py @@ -1,4 +1,5 @@ import unittest +from urllib.parse import urlparse from Testing.ZopeTestCase import FunctionalTestCase @@ -122,6 +123,45 @@ def test_ZopeVersion(self): self.assertEqual(app.ZopeVersion(major=True), int(pkg_version.split('.')[0])) + def test_getZMIMainFrameTarget(self): + app = self._makeOne() + + for URL1 in ('http://nohost', 'https://nohost/some/path'): + request = {'URL1': URL1} + parsed_url1 = urlparse(URL1) + + # No came_from at all + self.assertEqual(app.getZMIMainFrameTarget(request), + f'{URL1}/manage_workspace') + + # Empty came_from + request['came_from'] = '' + self.assertEqual(app.getZMIMainFrameTarget(request), + f'{URL1}/manage_workspace') + request['came_from'] = None + self.assertEqual(app.getZMIMainFrameTarget(request), + f'{URL1}/manage_workspace') + + # Local (path only) came_from + request['came_from'] = '/new/path' + self.assertEqual(app.getZMIMainFrameTarget(request), + f'/new/path') + + # came_from URL outside our own server + request['came_from'] = 'https://www.zope.dev/index.html' + self.assertEqual(app.getZMIMainFrameTarget(request), + f'{URL1}/manage_workspace') + + # came_from with wrong scheme + request['came_from'] = URL1.replace('http', 'ftp') + self.assertEqual(app.getZMIMainFrameTarget(request), + f'{URL1}/manage_workspace') + + # acceptable came_from + request['came_from'] = f'{URL1}/added/path' + self.assertEqual(app.getZMIMainFrameTarget(request), + f'{URL1}/added/path') + class ApplicationPublishTests(FunctionalTestCase): From e5f28fd2b1ad9a0a906f31c4b9afb0d5f969c418 Mon Sep 17 00:00:00 2001 From: Jens Vagelpohl Date: Sat, 9 Sep 2023 10:56:40 +0200 Subject: [PATCH 2/2] - lint fixes --- src/OFS/Application.py | 4 ++-- src/OFS/tests/testApplication.py | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/OFS/Application.py b/src/OFS/Application.py index 5147894b7f..2b3212d3fe 100644 --- a/src/OFS/Application.py +++ b/src/OFS/Application.py @@ -128,8 +128,8 @@ def getZMIMainFrameTarget(self, REQUEST): # Only allow a passed-in ``came_from`` URL if it is local (just a path) # or if the URL scheme and hostname are the same as our own if (not parsed_came_from.scheme and not parsed_came_from.netloc) or \ - (parsed_parent_url.scheme == parsed_came_from.scheme and - parsed_parent_url.netloc == parsed_came_from.netloc): + (parsed_parent_url.scheme == parsed_came_from.scheme + and parsed_parent_url.netloc == parsed_came_from.netloc): return came_from return default diff --git a/src/OFS/tests/testApplication.py b/src/OFS/tests/testApplication.py index 24a6005270..462e89c3b7 100644 --- a/src/OFS/tests/testApplication.py +++ b/src/OFS/tests/testApplication.py @@ -1,5 +1,4 @@ import unittest -from urllib.parse import urlparse from Testing.ZopeTestCase import FunctionalTestCase @@ -128,7 +127,6 @@ def test_getZMIMainFrameTarget(self): for URL1 in ('http://nohost', 'https://nohost/some/path'): request = {'URL1': URL1} - parsed_url1 = urlparse(URL1) # No came_from at all self.assertEqual(app.getZMIMainFrameTarget(request), @@ -145,7 +143,7 @@ def test_getZMIMainFrameTarget(self): # Local (path only) came_from request['came_from'] = '/new/path' self.assertEqual(app.getZMIMainFrameTarget(request), - f'/new/path') + '/new/path') # came_from URL outside our own server request['came_from'] = 'https://www.zope.dev/index.html'