CVE-2021-32674: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: Zope is an open-source web application server. This advisory extends the previous advisory at https://github.com/zopefoundation/Zope/security/advisories/GHSA-5pr9-v234-jw36 with additional cases of TAL expression traversal vulnerabilities. Most Python modules are not available for using in TAL expressions that you can add through-the-web, for example in Zope Page Templates. This restriction avoids file system access, for example via the 'os' module. But some of the untrusted modules are available indirectly through Python modules that are available for direct use. By default, you need to have the Manager role to add or edit Zope Page Templates through the web. Only sites that allow untrusted users to add/edit Zope Page Templates through the web are at risk. The problem has been fixed in Zope 5.2.1 and 4.6.1. The workaround is the same as for https://github.com/zopefoundation/Zope/security/advisories/GHSA-5pr9-v234-jw36: A site administrator can restrict adding/editing Zope Page Templates through the web using the standard Zope user/role permission mechanisms. Untrusted users should not be assigned the Zope Manager role and adding/editing Zope Page Templates through the web should be restricted to trusted users only.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
CVSS v3 base score: 8.8
Published: 2021-06-08T18:15Z
Repository: https://github.com/zopefoundation/Zope
Language: Python
Files changed: CHANGES.rst, src/OFS/zpt/main.zpt, src/Products/PageTemplates/Expressions.py, src/Products/PageTemplates/expression.py, src/Products/PageTemplates/tests/testExpressions.py, src/Products/PageTemplates/tests/testHTMLTests.py
Commit date: 2021-06-08 09:48:10 +0200
Commit message: Merge pull request from GHSA-rpcg-f9q6-2mq6

* - Prevent traversal through authorized Python modules in TAL expressions

* - try a more generic solution that doesn't use special casing

* - add suggestions from Maurits

* - integrate remaining suggestions from Maurits
Diff: diff --git a/CHANGES.rst b/CHANGES.rst
index 15308f6e33..0544e7c3e0 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -9,15 +9,18 @@ https://github.com/zopefoundation/Zope/blob/4.x/CHANGES.rst
 
 
 5.2.1 (unreleased)
 ------------------
 
-- Update to newest compatible versions of dependencies.
+- Prevent unauthorized traversal through authorized Python modules in
+  TAL expressions
 
 - Facelift the Zope logo.
   (`#973 <https://github.com/zopefoundation/Zope/issues/973>`_)
 
+- Update to newest compatible versions of dependencies.
+
 
 5.2 (2021-05-21)
 ----------------
 
 - Prevent traversal to names starting with ``_`` in TAL expressions
diff --git a/src/OFS/zpt/main.zpt b/src/OFS/zpt/main.zpt
index c772848cce..e3d4c814b5 100644
--- a/src/OFS/zpt/main.zpt
+++ b/src/OFS/zpt/main.zpt
@@ -3,11 +3,11 @@
 <tal:tabs replace="structure here/manage_tabs" />
 
 <main class="container-fluid">
   <form id="objectItems" name="objectItems" method="post"
         tal:define="has_order_support python:getattr(here.aq_explicit, 'has_order_support', 0);
-                    sm modules/AccessControl/SecurityManagement/getSecurityManager;
+                    sm modules/AccessControl/getSecurityManager;
                     default_sort python: 'position' if has_order_support else 'id';
                     skey python:request.get('skey',default_sort);
                     rkey python:request.get('rkey','asc');
                     rkey_alt python:'desc' if rkey=='asc' else 'asc';
                     rkey_alt_up rkey_alt/upper;
diff --git a/src/Products/PageTemplates/Expressions.py b/src/Products/PageTemplates/Expressions.py
index 66a748e0b1..39102017c1 100644
--- a/src/Products/PageTemplates/Expressions.py
+++ b/src/Products/PageTemplates/Expressions.py
@@ -19,10 +19,11 @@ for Python expressions, string literals, and paths.
 import logging
 import warnings
 
 import OFS.interfaces
 from AccessControl import safe_builtins
+from AccessControl.SecurityManagement import getSecurityManager
 from Acquisition import aq_base
 from MultiMapping import MultiMapping
 from zExceptions import NotFound
 from zExceptions import Unauthorized
 from zope.component import queryUtility
@@ -68,28 +69,47 @@ def boboAwareZopeTraverse(object, path_items, econtext):
     This uses zope.traversing path traversal where possible and interacts
     correctly with objects providing OFS.interface.ITraversable when
     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)
         else:
-            object = traversePathElement(object, name, path_items,
-                                         request=request)
+            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:
+                validate(object, object, name, found)
+                object = found
+            except Unauthorized:
+                # Convert Unauthorized to prevent information disclosures
+                raise NotFound(name)
+
     return object
 
 
 def trustedBoboAwareZopeTraverse(object, path_items, econtext):
     """Traverses a sequence of names, first trying attributes then items.
diff --git a/src/Products/PageTemplates/expression.py b/src/Products/PageTemplates/expression.py
index bca086492a..da01002c60 100644
--- a/src/Products/PageTemplates/expression.py
+++ b/src/Products/PageTemplates/expression.py
@@ -8,10 +8,11 @@ from chameleon.astutil import Static
 from chameleon.astutil import Symbol
 from chameleon.codegen import template
 from chameleon.tales import NotExpr
 from chameleon.tales import StringExpr
 
+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_iter
 from AccessControl.ZopeGuards import protected_inplacevar
@@ -55,28 +56,53 @@ class BoboAwareZopeTraverse:
 
     @classmethod
     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)
             else:
-                base = traversePathElement(base, name, path_items,
-                                           request=request)
+                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:
+                    validate(base, base, name, found)
+                    base = found
+                except Unauthorized:
+                    # Convert Unauthorized to prevent information disclosures
+                    raise NotFound(name)
 
         return base
 
     def __call__(self, base, econtext, call, path_items):
         request = econtext.get('request')
diff --git a/src/Products/PageTemplates/tests/testExpressions.py b/src/Products/PageTemplates/tests/testExpressions.py
index f877cfb877..a3983ce368 100644
--- a/src/Products/PageTemplates/tests/testExpressions.py
+++ b/src/Products/PageTemplates/tests/testExpressions.py
@@ -2,10 +2,11 @@ import unittest
 import warnings
 
 from AccessControl import safe_builtins
 from zExceptions import NotFound
 from zope.component.testing import PlacelessSetup
+from zope.location.interfaces import LocationError
 
 
 class EngineTestsBase(PlacelessSetup):
 
     def setUp(self):
@@ -231,14 +232,14 @@ class UntrustedEngineTests(EngineTestsBase, unittest.TestCase):
         ec = self._makeContext()
 
         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")
 
 
 class TrustedEngineTests(EngineTestsBase, unittest.TestCase):
 
diff --git a/src/Products/PageTemplates/tests/testHTMLTests.py b/src/Products/PageTemplates/tests/testHTMLTests.py
index 79ee58b3bc..47a359d449 100644
--- a/src/Products/PageTemplates/tests/testHTMLTests.py
+++ b/src/Products/PageTemplates/tests/testHTMLTests.py
@@ -24,21 +24,27 @@ from Products.PageTemplates.PageTemplate import PageTemplate
 from Products.PageTemplates.tests import util
 from Products.PageTemplates.unicodeconflictresolver import \
     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
 
 
 class AqPageTemplate(Implicit, PageTemplate):
     pass
 
 
+class AqZopePageTemplate(Implicit, ZopePageTemplate):
+    pass
+
+
 class Folder(util.Base):
     pass
 
 
 class UnitTestSecurityPolicy:
@@ -72,10 +78,11 @@ class HTMLTests(zope.component.testing.PlacelessSetup, unittest.TestCase):
                        IUnicodeEncodingConflictResolver)
 
         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.
 
     def tearDown(self):
@@ -224,11 +231,47 @@ class HTMLTests(zope.component.testing.PlacelessSetup, unittest.TestCase):
         t.write('<p tal:define="p context/__class__" />')
         with self.assertRaises(NotFound):
             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, LocationError)):
+            t()
+
+    def test_module_traversal(self):
+        t = self.folder.z
+
+        # Need to reset to the standard security policy so AccessControl
+        # checks are actually performed. The test setup initializes
+        # a policy that circumvents those checks.
+        SecurityManager.setSecurityPolicy(self.oldPolicy)
+        noSecurityManager()
+
+        # The getSecurityManager function is explicitly allowed
+        content = ('<p tal:define="a nocall:%s"'
+                   '   tal:content="python: a().getUser().getUserName()"/>')
+        t.write(content % 'modules/AccessControl/getSecurityManager')
+        self.assertEqual(t(), '<p>Anonymous User</p>')
+
+        # Anything else should be unreachable and raise NotFound:
+        # Direct access through AccessControl
+        t.write('<p tal:define="a nocall:modules/AccessControl/users"/>')
+        with self.assertRaises(NotFound):
+            t()
+
+        # Indirect access through an intermediary variable
+        content = ('<p tal:define="mod nocall:modules/AccessControl;'
+                   '               must_fail nocall:mod/users"/>')
+        t.write(content)
+        with self.assertRaises(NotFound):
+            t()
+
+        # Indirect access through an intermediary variable and a dictionary
+        content = ('<p tal:define="mod nocall:modules/AccessControl;'
+                   '               a_dict python: {\'unsafe\': mod};'
+                   '               must_fail nocall: a_dict/unsafe/users"/>')
+        t.write(content)
         with self.assertRaises(NotFound):
             t()

Vulnerable code: - Update to newest compatible versions of dependencies.
                    sm modules/AccessControl/SecurityManagement/getSecurityManager;
        if name == '_':
            warnings.warn('Traversing to the name `_` is deprecated '
                          'and will be removed in Zope 6.',
                          DeprecationWarning)
        elif name.startswith('_'):
            raise NotFound(name)

            object = traversePathElement(object, name, path_items,
                                         request=request)
            if name == '_':
                warnings.warn('Traversing to the name `_` is deprecated '
                              'and will be removed in Zope 6.',
                              DeprecationWarning)
            elif name.startswith('_'):
                raise NotFound(name)

                base = traversePathElement(base, name, path_items,
                                           request=request)
        with self.assertRaises(NotFound):
        with self.assertRaises(NotFound):
        with self.assertRaises(NotFound):
Fixed code: - Prevent unauthorized traversal through authorized Python modules in
  TAL expressions
- Update to newest compatible versions of dependencies.

                    sm modules/AccessControl/getSecurityManager;
from AccessControl.SecurityManagement import getSecurityManager
    validate = getSecurityManager().validate
            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:
                validate(object, object, name, found)
                object = found
            except Unauthorized:
                # Convert Unauthorized to prevent information disclosures
                raise NotFound(name)

from AccessControl.SecurityManagement import getSecurityManager
        validate = getSecurityManager().validate
                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:
                    validate(base, base, name, found)
                    base = found
                except Unauthorized:
                    # Convert Unauthorized to prevent information disclosures
                    raise NotFound(name)
from zope.location.interfaces import LocationError
        with self.assertRaises((NotFound, LocationError)):
        with self.assertRaises((NotFound, LocationError)):
from Products.PageTemplates.ZopePageTemplate import ZopePageTemplate
from zope.location.interfaces import LocationError
class AqZopePageTemplate(Implicit, ZopePageTemplate):
    pass


        f.z = AqZopePageTemplate('testing')
        with self.assertRaises((NotFound, LocationError)):
        with self.assertRaises((NotFound, LocationError)):
            t()

    def test_module_traversal(self):
        t = self.folder.z

        # Need to reset to the standard security policy so AccessControl
        # checks are actually performed. The test setup initializes
        # a policy that circumvents those checks.
        SecurityManager.setSecurityPolicy(self.oldPolicy)
        noSecurityManager()

        # The getSecurityManager function is explicitly allowed
        content = ('<p tal:define="a nocall:%s"'
                   '   tal:content="python: a().getUser().getUserName()"/>')
        t.write(content % 'modules/AccessControl/getSecurityManager')
        self.assertEqual(t(), '<p>Anonymous User</p>')

        # Anything else should be unreachable and raise NotFound:
        # Direct access through AccessControl
        t.write('<p tal:define="a nocall:modules/AccessControl/users"/>')
        with self.assertRaises(NotFound):
            t()

        # Indirect access through an intermediary variable
        content = ('<p tal:define="mod nocall:modules/AccessControl;'
                   '               must_fail nocall:mod/users"/>')
        t.write(content)
        with self.assertRaises(NotFound):
            t()

        # Indirect access through an intermediary variable and a dictionary
        content = ('<p tal:define="mod nocall:modules/AccessControl;'
                   '               a_dict python: {\'unsafe\': mod};'
                   '               must_fail nocall: a_dict/unsafe/users"/>')
        t.write(content)