CVE-2021-32633: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: Zope is an open-source web application server. In Zope versions prior to 4.6 and 5.2, users can access untrusted modules indirectly through Python modules that are available for direct use. By default, only users with the Manager role can add or edit Zope Page Templates through the web, but sites that allow untrusted users to add/edit Zope Page Templates through the web are at risk from this vulnerability. The problem has been fixed in Zope 5.2 and 4.6. As a workaround, 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-05-21T14:15Z
Repository: https://github.com/zopefoundation/Zope
Language: Python
Files changed: CHANGES.rst, src/Products/PageTemplates/Expressions.py, src/Products/PageTemplates/expression.py, src/Products/PageTemplates/tests/input/CheckPathTraverse.html, src/Products/PageTemplates/tests/output/CheckPathTraverse.html, src/Products/PageTemplates/tests/testChameleonTalesExpressions.py, src/Products/PageTemplates/tests/testExpressions.py, src/Products/PageTemplates/tests/testHTMLTests.py
Commit date: 2021-05-21 09:11:02 +0200
Commit message: Merge pull request from GHSA-5pr9-v234-jw36

* - Prevent traversal to names starting with ``_`` in TAL expressions

* - include the expressions module

* - simplify test

* - lint fix

* - add DeprecationWarnings for traversal to `_`

* Update src/Products/PageTemplates/tests/testChameleonTalesExpressions.py

Co-authored-by: Michael Howitz <mh@gocept.com>

* - add missing import

* - added tests for the ``traverse`` method fix

* - change control flow

Co-authored-by: Michael Howitz <mh@gocept.com>
Diff: diff --git a/CHANGES.rst b/CHANGES.rst
index 942752d1d..b78ce36f0 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -9,10 +9,13 @@ https://github.com/zopefoundation/Zope/blob/4.x/CHANGES.rst
 
 
 5.2 (unreleased)
 ----------------
 
+- Prevent traversal to names starting with ``_`` in TAL expressions
+  and fix path expressions for the ``chameleon.tales`` expression engine.
+
 - Provide friendlier ZMI error message for the Transaction Undo form
   (`#964 <https://github.com/zopefoundation/Zope/issues/964>`_)
 
 - Updated/fixed the poll application tutorial in the Zope Developers Guide
   (`#958 <https://github.com/zopefoundation/Zope/issues/958>`_)
diff --git a/src/Products/PageTemplates/Expressions.py b/src/Products/PageTemplates/Expressions.py
index 2a538391e..66a748e0b 100644
--- a/src/Products/PageTemplates/Expressions.py
+++ b/src/Products/PageTemplates/Expressions.py
@@ -15,10 +15,11 @@
 Page Template-specific implementation of TALES, with handlers
 for Python expressions, string literals, and paths.
 """
 
 import logging
+import warnings
 
 import OFS.interfaces
 from AccessControl import safe_builtins
 from Acquisition import aq_base
 from MultiMapping import MultiMapping
@@ -72,10 +73,18 @@ def boboAwareZopeTraverse(object, path_items, econtext):
     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)
diff --git a/src/Products/PageTemplates/expression.py b/src/Products/PageTemplates/expression.py
index f65132220..bca086492 100644
--- a/src/Products/PageTemplates/expression.py
+++ b/src/Products/PageTemplates/expression.py
@@ -1,7 +1,8 @@
 """``chameleon.tales`` expressions."""
 
+import warnings
 from ast import NodeTransformer
 from ast import parse
 
 from chameleon.astutil import Static
 from chameleon.astutil import Symbol
@@ -59,12 +60,20 @@ class BoboAwareZopeTraverse:
         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.traverseMethod)(name)
+                base = getattr(base, cls.traverse_method)(name)
             else:
                 base = traversePathElement(base, name, path_items,
                                            request=request)
 
         return base
diff --git a/src/Products/PageTemplates/tests/input/CheckPathTraverse.html b/src/Products/PageTemplates/tests/input/CheckPathTraverse.html
new file mode 100644
index 000000000..cd47c46b7
--- /dev/null
+++ b/src/Products/PageTemplates/tests/input/CheckPathTraverse.html
@@ -0,0 +1,5 @@
+<html>
+<body>
+   <div tal:content="context/laf"></div>
+</body>
+</html>
diff --git a/src/Products/PageTemplates/tests/output/CheckPathTraverse.html b/src/Products/PageTemplates/tests/output/CheckPathTraverse.html
new file mode 100644
index 000000000..09c100540
--- /dev/null
+++ b/src/Products/PageTemplates/tests/output/CheckPathTraverse.html
@@ -0,0 +1,5 @@
+<html>
+<body>
+   <div>ok</div>
+</body>
+</html>
diff --git a/src/Products/PageTemplates/tests/testChameleonTalesExpressions.py b/src/Products/PageTemplates/tests/testChameleonTalesExpressions.py
index aa71d6513..fd25a55a3 100644
--- a/src/Products/PageTemplates/tests/testChameleonTalesExpressions.py
+++ b/src/Products/PageTemplates/tests/testChameleonTalesExpressions.py
@@ -1,5 +1,7 @@
+import unittest
+
 from ..expression import getEngine
 from . import testHTMLTests
 
 
 class ChameleonAqPageTemplate(testHTMLTests.AqPageTemplate):
@@ -17,5 +19,10 @@ class ChameleonTalesExpressionTests(testHTMLTests.HTMLTests):
     # override ``PREFIX`` to be able to account for
     #   small differences between ``zope.tales`` and ``chameleon.tales``
     #   expressions (e.g. the ``zope.tales`` ``not`` expression
     #   returns ``int``, that of ``chameleon.tales`` ``bool``
     PREFIX = "CH_"
+
+    @unittest.skip('The test in the base class relies on a Zope context with'
+                   ' the "random" module available in expressions')
+    def test_underscore_traversal(self):
+        pass
diff --git a/src/Products/PageTemplates/tests/testExpressions.py b/src/Products/PageTemplates/tests/testExpressions.py
index d2cb1672e..f877cfb87 100644
--- a/src/Products/PageTemplates/tests/testExpressions.py
+++ b/src/Products/PageTemplates/tests/testExpressions.py
@@ -1,8 +1,10 @@
 import unittest
+import warnings
 
 from AccessControl import safe_builtins
+from zExceptions import NotFound
 from zope.component.testing import PlacelessSetup
 
 
 class EngineTestsBase(PlacelessSetup):
 
@@ -104,12 +106,16 @@ class EngineTestsBase(PlacelessSetup):
     def test_evaluate_alternative_first_missing(self):
         ec = self._makeContext()
         self.assertTrue(ec.evaluate('x | nothing') is None)
 
     def test_evaluate_dict_key_as_underscore(self):
+        # Traversing to the name `_` will raise a DeprecationWarning
+        # because it will go away in Zope 6.
         ec = self._makeContext()
-        self.assertEqual(ec.evaluate('d/_'), 'under')
+        with warnings.catch_warnings():
+            warnings.simplefilter('ignore')
+            self.assertEqual(ec.evaluate('d/_'), 'under')
 
     def test_evaluate_dict_with_key_from_expansion(self):
         ec = self._makeContext()
         self.assertEqual(ec.evaluate('d/?blank'), 'blank')
 
@@ -218,10 +224,23 @@ class UntrustedEngineTests(EngineTestsBase, unittest.TestCase):
 
     def test_list_in_path_expr(self):
         ec = self._makeContext()
         self.assertIs(ec.evaluate('nocall: list'), safe_builtins["list"])
 
+    def test_underscore_traversal(self):
+        # Prevent traversal to names starting with an underscore (_)
+        ec = self._makeContext()
+
+        with self.assertRaises(NotFound):
+            ec.evaluate("context/__class__")
+
+        with self.assertRaises(NotFound):
+            ec.evaluate("nocall: random/_itertools/repeat")
+
+        with self.assertRaises(NotFound):
+            ec.evaluate("random/_itertools/repeat/foobar")
+
 
 class TrustedEngineTests(EngineTestsBase, unittest.TestCase):
 
     def _makeEngine(self):
         from Products.PageTemplates.Expressions import createTrustedZopeEngine
diff --git a/src/Products/PageTemplates/tests/testHTMLTests.py b/src/Products/PageTemplates/tests/testHTMLTests.py
index e113095b7..79ee58b3b 100644
--- a/src/Products/PageTemplates/tests/testHTMLTests.py
+++ b/src/Products/PageTemplates/tests/testHTMLTests.py
@@ -24,10 +24,11 @@ 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 zExceptions import NotFound
 from zope.component import provideUtility
 from zope.traversing.adapters import DefaultTraversable
 
 from .util import useChameleonEngine
 
@@ -153,10 +154,19 @@ class HTMLTests(zope.component.testing.PlacelessSetup, unittest.TestCase):
         self.assert_expected(self.folder.t, 'CheckPathNothing.html')
 
     def testPathAlt(self):
         self.assert_expected(self.folder.t, 'CheckPathAlt.html')
 
+    def testPathTraverse(self):
+        # need to perform this test with a "real" folder
+        from OFS.Folder import Folder
+        f = self.folder
+        self.folder = Folder()
+        self.folder.t, self.folder.laf = f.t, f.laf
+        self.folder.laf.write('ok')
+        self.assert_expected(self.folder.t, 'CheckPathTraverse.html')
+
     def testBatchIteration(self):
         self.assert_expected(self.folder.t, 'CheckBatchIteration.html')
 
     def testUnicodeInserts(self):
         self.assert_expected_unicode(self.folder.t, 'CheckUnicodeInserts.html')
@@ -205,5 +215,20 @@ class HTMLTests(zope.component.testing.PlacelessSetup, unittest.TestCase):
     def test_unicode_conflict_resolution(self):
         # override with the more "demanding" resolver
         provideUtility(PreferredCharsetResolver)
         t = PageTemplate()
         self.assert_expected(t, 'UnicodeResolution.html')
+
+    def test_underscore_traversal(self):
+        t = self.folder.t
+
+        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):
+            t()
+
+        t.write('<p tal:content="random/_itertools/repeat/foobar"/>')
+        with self.assertRaises(NotFound):
+            t()

Vulnerable code:                 base = getattr(base, cls.traverseMethod)(name)
        self.assertEqual(ec.evaluate('d/_'), 'under')
Fixed code: - Prevent traversal to names starting with ``_`` in TAL expressions
  and fix path expressions for the ``chameleon.tales`` expression engine.

import warnings

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

import warnings

            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 = getattr(base, cls.traverse_method)(name)
<html>
<body>
   <div tal:content="context/laf"></div>
</body>
</html>
<html>
<body>
   <div>ok</div>
</body>
</html>
import unittest


    @unittest.skip('The test in the base class relies on a Zope context with'
                   ' the "random" module available in expressions')
    def test_underscore_traversal(self):
        pass
import warnings
from zExceptions import NotFound
        # Traversing to the name `_` will raise a DeprecationWarning
        # because it will go away in Zope 6.
        with warnings.catch_warnings():
            warnings.simplefilter('ignore')
            self.assertEqual(ec.evaluate('d/_'), 'under')
    def test_underscore_traversal(self):
        # Prevent traversal to names starting with an underscore (_)
        ec = self._makeContext()

        with self.assertRaises(NotFound):
            ec.evaluate("context/__class__")

        with self.assertRaises(NotFound):
            ec.evaluate("nocall: random/_itertools/repeat")

        with self.assertRaises(NotFound):
            ec.evaluate("random/_itertools/repeat/foobar")

from zExceptions import NotFound
    def testPathTraverse(self):
        # need to perform this test with a "real" folder
        from OFS.Folder import Folder
        f = self.folder
        self.folder = Folder()
        self.folder.t, self.folder.laf = f.t, f.laf
        self.folder.laf.write('ok')
        self.assert_expected(self.folder.t, 'CheckPathTraverse.html')


    def test_underscore_traversal(self):
        t = self.folder.t

        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):
            t()

        t.write('<p tal:content="random/_itertools/repeat/foobar"/>')
        with self.assertRaises(NotFound):
            t()