Skip to content
Permalink
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
dataflake and icemac committed May 21, 2021
1 parent 1746d5a commit 1f8456b
Show file tree
Hide file tree
Showing 8 changed files with 84 additions and 2 deletions.
3 changes: 3 additions & 0 deletions CHANGES.rst
Expand Up @@ -11,6 +11,9 @@ 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>`_)

Expand Down
9 changes: 9 additions & 0 deletions src/Products/PageTemplates/Expressions.py
Expand Up @@ -17,6 +17,7 @@
"""

import logging
import warnings

import OFS.interfaces
from AccessControl import safe_builtins
Expand Down Expand Up @@ -74,6 +75,14 @@ def boboAwareZopeTraverse(object, path_items, econtext):

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:
Expand Down
11 changes: 10 additions & 1 deletion src/Products/PageTemplates/expression.py
@@ -1,5 +1,6 @@
"""``chameleon.tales`` expressions."""

import warnings
from ast import NodeTransformer
from ast import parse

Expand Down Expand Up @@ -61,8 +62,16 @@ def traverse(cls, base, request, path_items):

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)
Expand Down
5 changes: 5 additions & 0 deletions src/Products/PageTemplates/tests/input/CheckPathTraverse.html
@@ -0,0 +1,5 @@
<html>
<body>
<div tal:content="context/laf"></div>
</body>
</html>
@@ -0,0 +1,5 @@
<html>
<body>
<div>ok</div>
</body>
</html>
@@ -1,3 +1,5 @@
import unittest

from ..expression import getEngine
from . import testHTMLTests

Expand All @@ -19,3 +21,8 @@ def setUp(self):
# 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
21 changes: 20 additions & 1 deletion src/Products/PageTemplates/tests/testExpressions.py
@@ -1,6 +1,8 @@
import unittest
import warnings

from AccessControl import safe_builtins
from zExceptions import NotFound
from zope.component.testing import PlacelessSetup


Expand Down Expand Up @@ -106,8 +108,12 @@ def test_evaluate_alternative_first_missing(self):
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()
Expand Down Expand Up @@ -220,6 +226,19 @@ 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):

Expand Down
25 changes: 25 additions & 0 deletions src/Products/PageTemplates/tests/testHTMLTests.py
Expand Up @@ -26,6 +26,7 @@
DefaultUnicodeEncodingConflictResolver
from Products.PageTemplates.unicodeconflictresolver import \
PreferredCharsetResolver
from zExceptions import NotFound
from zope.component import provideUtility
from zope.traversing.adapters import DefaultTraversable

Expand Down Expand Up @@ -155,6 +156,15 @@ def testPathNothing(self):
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')

Expand Down Expand Up @@ -207,3 +217,18 @@ def test_unicode_conflict_resolution(self):
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()

0 comments on commit 1f8456b

Please sign in to comment.