- Backport stricter traversal checks from Zope 5 (#983)

CHANGES.rst

@@ -10,6 +10,8 @@ https://zope.readthedocs.io/en/2.13/CHANGES.html
 4.6.2 (unreleased)
 ------------------
 
+- Backport stricter traversal checks from Zope 5
+
 - Update dependencies to the latest releases that still support Python 2.
 
 

src/Products/PageTemplates/Expressions.py

@@ -17,15 +17,14 @@
 """
 
 import logging
-import types
 import warnings
 
 from six import binary_type
 from six import text_type
 
 import OFS.interfaces
 from AccessControl import safe_builtins
-from AccessControl.ZopeGuards import guarded_import
+from AccessControl.SecurityManagement import getSecurityManager
 from Acquisition import aq_base
 from MultiMapping import MultiMapping
 from zExceptions import NotFound
@@ -75,33 +74,43 @@ def boboAwareZopeTraverse(object, path_items, econtext):
     necessary (bobo-awareness).
     """
     request = getattr(econtext, 'request', None)
+    validate = getSecurityManager().validate
     path_items = list(path_items)
     path_items.reverse()
 
     while path_items:
         name = path_items.pop()
 
-        if name == '_':
-            warnings.warn('Traversing to the name `_` is deprecated '
-                          'and will be removed in Zope 6.',
-                          DeprecationWarning)
-        elif name.startswith('_'):
-            raise NotFound(name)
-
         if OFS.interfaces.ITraversable.providedBy(object):
             object = object.restrictedTraverse(name)
-        elif isinstance(object, types.ModuleType):
+        else:
+            found = traversePathElement(object, name, path_items,
+                                        request=request)
+
+            # Special backwards compatibility exception for the name ``_``,
+            # which was often used for translation message factories.
+            # Allow and continue traversal.
+            if name == '_':
+                warnings.warn('Traversing to the name `_` is deprecated '
+                              'and will be removed in Zope 6.',
+                              DeprecationWarning)
+                object = found
+                continue
+
+            # All other names starting with ``_`` are disallowed.
+            # This emulates what restrictedTraverse does.
+            if name.startswith('_'):
+                raise NotFound(name)
+
+            # traversePathElement doesn't apply any Zope security policy,
+            # so we validate access explicitly here.
             try:
-                # guarded_import will do all necessary security checking
-                # but will not return the imported item itself.
-                guarded_import(object.__name__, fromlist=[name])
-                object = getattr(object, name)
+                validate(object, object, name, found)
+                object = found
             except Unauthorized:
                 # Convert Unauthorized to prevent information disclosures
                 raise NotFound(name)
-        else:
-            object = traversePathElement(object, name, path_items,
-                                         request=request)
+
     return object
 
 

src/Products/PageTemplates/expression.py

@@ -3,7 +3,6 @@
 import warnings
 from ast import NodeTransformer
 from ast import parse
-from types import ModuleType
 
 from chameleon.astutil import Static
 from chameleon.astutil import Symbol
@@ -12,10 +11,10 @@
 from chameleon.tales import StringExpr
 from six import class_types
 
+from AccessControl.SecurityManagement import getSecurityManager
 from AccessControl.ZopeGuards import guarded_apply
 from AccessControl.ZopeGuards import guarded_getattr
 from AccessControl.ZopeGuards import guarded_getitem
-from AccessControl.ZopeGuards import guarded_import
 from AccessControl.ZopeGuards import guarded_iter
 from AccessControl.ZopeGuards import protected_inplacevar
 from OFS.interfaces import ITraversable
@@ -60,33 +59,49 @@ class BoboAwareZopeTraverse(object):
     def traverse(cls, base, request, path_items):
         """See ``zope.app.pagetemplate.engine``."""
 
+        validate = getSecurityManager().validate
         path_items = list(path_items)
         path_items.reverse()
 
         while path_items:
             name = path_items.pop()
 
-            if name == '_':
-                warnings.warn('Traversing to the name `_` is deprecated '
-                              'and will be removed in Zope 6.',
-                              DeprecationWarning)
-            elif name.startswith('_'):
-                raise NotFound(name)
-
             if ITraversable.providedBy(base):
                 base = getattr(base, cls.traverse_method)(name)
-            elif isinstance(base, ModuleType):
+            else:
+                found = traversePathElement(base, name, path_items,
+                                            request=request)
+
+                # If traverse_method is something other than
+                # ``restrictedTraverse`` then traversal is assumed to be
+                # unrestricted. This emulates ``unrestrictedTraverse``
+                if cls.traverse_method != 'restrictedTraverse':
+                    base = found
+                    continue
+
+                # Special backwards compatibility exception for the name ``_``,
+                # which was often used for translation message factories.
+                # Allow and continue traversal.
+                if name == '_':
+                    warnings.warn('Traversing to the name `_` is deprecated '
+                                  'and will be removed in Zope 6.',
+                                  DeprecationWarning)
+                    base = found
+                    continue
+
+                # All other names starting with ``_`` are disallowed.
+                # This emulates what restrictedTraverse does.
+                if name.startswith('_'):
+                    raise NotFound(name)
+
+                # traversePathElement doesn't apply any Zope security policy,
+                # so we validate access explicitly here.
                 try:
-                    # guarded_import will do all necessary security checking
-                    # but will not return the imported item itself.
-                    guarded_import(base.__name__, fromlist=[name])
-                    base = getattr(base, name)
+                    validate(base, base, name, found)
+                    base = found
                 except Unauthorized:
                     # Convert Unauthorized to prevent information disclosures
                     raise NotFound(name)
-            else:
-                base = traversePathElement(base, name, path_items,
-                                           request=request)
 
         return base
 

src/Products/PageTemplates/tests/testExpressions.py

@@ -8,6 +8,7 @@
 from AccessControl import safe_builtins
 from zExceptions import NotFound
 from zope.component.testing import PlacelessSetup
+from zope.location.interfaces import LocationError
 
 
 class EngineTestsBase(PlacelessSetup):
@@ -237,10 +238,10 @@ def test_underscore_traversal(self):
         with self.assertRaises(NotFound):
             ec.evaluate("context/__class__")
 
-        with self.assertRaises(NotFound):
+        with self.assertRaises((NotFound, LocationError)):
             ec.evaluate("nocall: random/_itertools/repeat")
 
-        with self.assertRaises(NotFound):
+        with self.assertRaises((NotFound, LocationError)):
             ec.evaluate("random/_itertools/repeat/foobar")
 
 

src/Products/PageTemplates/tests/testHTMLTests.py

@@ -27,8 +27,10 @@
     DefaultUnicodeEncodingConflictResolver
 from Products.PageTemplates.unicodeconflictresolver import \
     PreferredCharsetResolver
+from Products.PageTemplates.ZopePageTemplate import ZopePageTemplate
 from zExceptions import NotFound
 from zope.component import provideUtility
+from zope.location.interfaces import LocationError
 from zope.traversing.adapters import DefaultTraversable
 
 from .util import useChameleonEngine
@@ -38,6 +40,10 @@ class AqPageTemplate(Implicit, PageTemplate):
     pass
 
 
+class AqZopePageTemplate(Implicit, ZopePageTemplate):
+    pass
+
+
 class Folder(util.Base):
     pass
 
@@ -75,6 +81,7 @@ def setUp(self):
         self.folder = f = Folder()
         f.laf = AqPageTemplate()
         f.t = AqPageTemplate()
+        f.z = AqZopePageTemplate('testing')
         self.policy = UnitTestSecurityPolicy()
         self.oldPolicy = SecurityManager.setSecurityPolicy(self.policy)
         noSecurityManager()  # Use the new policy.
@@ -227,15 +234,15 @@ def test_underscore_traversal(self):
             t()
 
         t.write('<p tal:define="p nocall: random/_itertools/repeat"/>')
-        with self.assertRaises(NotFound):
+        with self.assertRaises((NotFound, LocationError)):
             t()
 
         t.write('<p tal:content="random/_itertools/repeat/foobar"/>')
-        with self.assertRaises(NotFound):
+        with self.assertRaises((NotFound, LocationError)):
             t()
 
     def test_module_traversal(self):
-        t = self.folder.t
+        t = self.folder.z
 
         # Need to reset to the standard security policy so AccessControl
         # checks are actually performed. The test setup initializes