diff --git a/src/OFS/Application.py b/src/OFS/Application.py index 2b3212d3fe..0bc289b3d0 100644 --- a/src/OFS/Application.py +++ b/src/OFS/Application.py @@ -24,6 +24,7 @@ from AccessControl.class_init import InitializeClass from AccessControl.Permission import ApplicationDefaultPermissions from AccessControl.Permissions import view_management_screens +from AccessControl.tainted import TaintedString from Acquisition import aq_base from App import FactoryDispatcher from App.ApplicationManager import ApplicationManager @@ -122,15 +123,30 @@ def getZMIMainFrameTarget(self, REQUEST): if not came_from: return default + # When came_from contains suspicious code, it will not be a string, + # but an instance of AccessControl.tainted.TaintedString. + # Passing this to urlparse, gives: + # AttributeError: 'str' object has no attribute 'decode' + # This is good, but let's check explicitly. + if isinstance(came_from, TaintedString): + return default + try: + parsed_came_from = urlparse(came_from) + except AttributeError: + 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 + if (parsed_parent_url.scheme == parsed_came_from.scheme and parsed_parent_url.netloc == parsed_came_from.netloc): return came_from + if (not parsed_came_from.scheme and not parsed_came_from.netloc): + # This is only a path. But some paths can be misinterpreted + # by browsers. + if parsed_came_from.path.startswith("//"): + return default + return came_from return default diff --git a/src/OFS/tests/testApplication.py b/src/OFS/tests/testApplication.py index 462e89c3b7..f7fc6bd8e4 100644 --- a/src/OFS/tests/testApplication.py +++ b/src/OFS/tests/testApplication.py @@ -1,5 +1,6 @@ import unittest +from AccessControl.tainted import TaintedString from Testing.ZopeTestCase import FunctionalTestCase @@ -145,6 +146,12 @@ def test_getZMIMainFrameTarget(self): self.assertEqual(app.getZMIMainFrameTarget(request), '/new/path') + # Tainted local path. came_from can be marked as 'tainted' if it + # suspicious contents. It is not accepted then. + request['came_from'] = TaintedString('/new/path') + self.assertEqual(app.getZMIMainFrameTarget(request), + f'{URL1}/manage_workspace') + # came_from URL outside our own server request['came_from'] = 'https://www.zope.dev/index.html' self.assertEqual(app.getZMIMainFrameTarget(request), @@ -160,6 +167,23 @@ def test_getZMIMainFrameTarget(self): self.assertEqual(app.getZMIMainFrameTarget(request), f'{URL1}/added/path') + # Anything beginning with '' + ) + self.assertEqual(app.getZMIMainFrameTarget(request), + f'{URL1}/manage_workspace') + + # double slashes as path should not be accepted. + # Try a few forms. + request['came_from'] = '//www.example.org' + self.assertEqual(app.getZMIMainFrameTarget(request), + f'{URL1}/manage_workspace') + request['came_from'] = '////www.example.org' + self.assertEqual(app.getZMIMainFrameTarget(request), + f'{URL1}/manage_workspace') + class ApplicationPublishTests(FunctionalTestCase):