Tighten down the ZMI frame source logic to only allow site-local sour…

…ces (#1151)

* - Tighten down the ZMI frame source logic to only allow site-local sources

* - lint fixes

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.
 

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',

src/App/dtml/manage.dtml

@@ -19,7 +19,7 @@
 		name="manage_menu"
 		marginwidth="2" marginheight="2"
 		/>
-	<frame src="<dtml-var "REQUEST.get('came_from',None)!=None and REQUEST.get('came_from',None) or '%s/manage_workspace'%(REQUEST.URL1)" html_quote>"
+	<frame src="<dtml-var "getZMIMainFrameTarget(REQUEST)" html_quote>"
 		name="manage_main"
 		marginwidth="2" marginheight="2"
 		/>

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)

src/OFS/tests/testApplication.py

@@ -122,6 +122,44 @@ 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}
+
+            # 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),
+                             '/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):