Test that str.format checks security for accessed keys and items.

The real fix is in the AccessControl package, which we checkout on a specific branch here. Part of PloneHotfix20171128.
zopefoundation · Dec 4, 2017 · bb1f3aa · bb1f3aa
1 parent 70b9014
commit bb1f3aa
Show file tree

Hide file tree

Showing 5 changed files with 154 additions and 10 deletions.
diff --git a/buildout.cfg b/buildout.cfg
@@ -23,6 +23,7 @@ parts =
     wsgi
 sources-dir = develop
 auto-checkout =
+    AccessControl
 versions = versions
 
 

diff --git a/doc/CHANGES.rst b/doc/CHANGES.rst
@@ -8,6 +8,10 @@ http://docs.zope.org/zope2/
 2.13.27 (unreleased)
 --------------------
 
+- Test that `str.format` checks security for accessed keys and items.
+  The real fix is in the AccessControl package.
+  Part of PloneHotfix20171128.
+
 - Made Redirect unavailable as url.  Part of PloneHotfix20171128.
 
 - Skip IPv6 tests on Travis, as it is not supported.

diff --git a/sources.cfg b/sources.cfg
@@ -3,7 +3,7 @@ github = git://github.com/zopefoundation
 github_push = git@github.com:zopefoundation
 
 [sources]
-AccessControl = git ${remotes:github}/AccessControl pushurl=${remotes:github_push}/AccessControl branch=2.13
+AccessControl = git ${remotes:github}/AccessControl pushurl=${remotes:github_push}/AccessControl branch=plone-hotfix20171128-format-213
 Acquisition = git ${remotes:github}/Acquisition pushurl=${remotes:github_push}/Acquisition branch=2.13
 DateTime = git ${remotes:github}/DateTime pushurl=${remotes:github_push}/DateTime branch=2.12
 DocumentTemplate = git ${remotes:github}/DocumentTemplate pushurl=${remotes:github_push}/DocumentTemplate branch=2.13

diff --git a/src/Zope2/App/tests/test_formatter.py b/src/Zope2/App/tests/test_formatter.py
@@ -4,14 +4,33 @@
 import unittest
 
 
-BAD_STR = """
+BAD_ATTR_STR = """
 <p tal:content="python:'class of {0} is {0.__class__}'.format(context)" />
 """
-BAD_UNICODE = """
+BAD_ATTR_UNICODE = """
 <p tal:content="python:u'class of {0} is {0.__class__}'.format(context)" />
 """
+BAD_KEY_STR = """
+<p tal:content="python:'access by key: {0[test_folder_1_]}'.format(context)" />
+"""
+BAD_KEY_UNICODE = """
+<p tal:content="python:u'access by key: {0[test_folder_1_]}'.format(context)" />
+"""
+BAD_ITEM_STR = """
+<p tal:content="python:'access by item: {0[0]}'.format(context)" />
+"""
+BAD_ITEM_UNICODE = """
+<p tal:content="python:u'access by item: {0[0]}'.format(context)" />
+"""
 GOOD_STR = '<p tal:content="python:(\'%s\' % context).lower()" />'
 GOOD_UNICODE = '<p tal:content="python:(\'%s\' % context).lower()" />'
+# Attribute access is not completely forbidden, it is simply checked.
+GOOD_FORMAT_ATTR_STR = """
+<p tal:content="python:'title of {0} is {0.title}'.format(context)" />
+"""
+GOOD_FORMAT_ATTR_UNICODE = """
+<p tal:content="python:u'title of {0} is {0.title}'.format(context)" />
+"""
 
 
 def noop(context=None):
@@ -26,34 +45,66 @@ def hack_pt(pt, context=None):
     pt.context = context
 
 
-class FormatterTest(unittest.TestCase):
+class UnauthorizedSecurityPolicy:
+    """Policy which denies every access."""
+
+    def validate(self, *args, **kw):
+        from AccessControl.unauthorized import Unauthorized
+        raise Unauthorized('Nothing is allowed!')
+
+
+class FormatterFunctionalTest(FunctionalTestCase):
 
-    def test_cook_zope2_page_templates_bad_str(self):
+    def test_cook_zope2_page_templates_bad_attr_str(self):
         from Products.PageTemplates.ZopePageTemplate import ZopePageTemplate
-        pt = ZopePageTemplate('mytemplate', BAD_STR)
+        pt = ZopePageTemplate('mytemplate', BAD_ATTR_STR)
         hack_pt(pt)
         self.assertRaises(Unauthorized, pt.pt_render)
+        hack_pt(pt, context=self.app)
+        self.assertRaises(Unauthorized, pt.pt_render)
 
-    def test_cook_zope2_page_templates_bad_unicode(self):
+    def test_cook_zope2_page_templates_bad_attr_unicode(self):
         from Products.PageTemplates.ZopePageTemplate import ZopePageTemplate
-        pt = ZopePageTemplate('mytemplate', BAD_UNICODE)
+        pt = ZopePageTemplate('mytemplate', BAD_ATTR_UNICODE)
         hack_pt(pt)
         self.assertRaises(Unauthorized, pt.pt_render)
+        hack_pt(pt, context=self.app)
+        self.assertRaises(Unauthorized, pt.pt_render)
 
     def test_cook_zope2_page_templates_good_str(self):
         from Products.PageTemplates.ZopePageTemplate import ZopePageTemplate
         pt = ZopePageTemplate('mytemplate', GOOD_STR)
         hack_pt(pt)
         self.assertEqual(pt.pt_render().strip(), '<p>none</p>')
+        hack_pt(pt, context=self.app)
+        self.assertEqual(
+            pt.pt_render().strip(), '<p>&lt;application at &gt;</p>')
 
     def test_cook_zope2_page_templates_good_unicode(self):
         from Products.PageTemplates.ZopePageTemplate import ZopePageTemplate
         pt = ZopePageTemplate('mytemplate', unicode(GOOD_UNICODE))
         hack_pt(pt)
         self.assertEqual(pt.pt_render().strip(), '<p>none</p>')
+        hack_pt(pt, self.app)
+        self.assertEqual(
+            pt.pt_render().strip(), '<p>&lt;application at &gt;</p>')
 
+    def test_cook_zope2_page_templates_good_format_attr_str(self):
+        from Products.PageTemplates.ZopePageTemplate import ZopePageTemplate
+        pt = ZopePageTemplate('mytemplate', GOOD_FORMAT_ATTR_STR)
+        hack_pt(pt, self.app)
+        self.assertEqual(
+            pt.pt_render().strip(),
+            '<p>title of &lt;Application at &gt; is Zope</p>')
+
+    def test_cook_zope2_page_templates_good_format_attr_unicode(self):
+        from Products.PageTemplates.ZopePageTemplate import ZopePageTemplate
+        pt = ZopePageTemplate('mytemplate', GOOD_FORMAT_ATTR_UNICODE)
+        hack_pt(pt, self.app)
+        self.assertEqual(
+            pt.pt_render().strip(),
+            '<p>title of &lt;Application at &gt; is Zope</p>')
 
-class FormatterFunctionalTest(FunctionalTestCase):
 
     def test_access_to_private_content_not_allowed_via_any_attribute(self):
         from Products.PageTemplates.ZopePageTemplate import ZopePageTemplate
@@ -87,9 +138,93 @@ def test_cook_zope3_page_templates_using_format(self):
         hack_pt(pt)
         # Need to pass a namespace.
         namespace = {'context': self.app}
+        # Even when one of the accessed items requires a role that we do
+        # not have, we get no Unauthorized, because this is a filesystem
+        # template.
+        self.app.test_folder_1_.__roles__ = ['Manager']
         self.assertEqual(
             pt.pt_render(namespace).strip(),
             u"<p>class of &lt;application at &gt; is "
             u"&lt;class 'ofs.application.application'&gt;</p>\n"
             u"<p>CLASS OF &lt;APPLICATION AT &gt; IS "
-            u"&lt;CLASS 'OFS.APPLICATION.APPLICATION'&gt;</p>")
+            u"&lt;CLASS 'OFS.APPLICATION.APPLICATION'&gt;</p>\n"
+            u"<p>{'foo': &lt;Folder at /test_folder_1_&gt;} has "
+            u"foo=&lt;Folder at test_folder_1_&gt;</p>\n"
+            u"<p>{'foo': &lt;Folder at /test_folder_1_&gt;} has "
+            u"foo=&lt;Folder at test_folder_1_&gt;</p>\n"
+            u"<p>[&lt;Folder at /test_folder_1_&gt;] has "
+            u"first item &lt;Folder at test_folder_1_&gt;</p>\n"
+            u"<p>[&lt;Folder at /test_folder_1_&gt;] has "
+            u"first item &lt;Folder at test_folder_1_&gt;</p>"
+        )
+
+    def test_cook_zope2_page_templates_bad_key_str(self):
+        from Products.PageTemplates.ZopePageTemplate import ZopePageTemplate
+        pt = ZopePageTemplate('mytemplate', BAD_KEY_STR)
+        hack_pt(pt, self.app)
+        self.assertEqual(
+            pt.pt_render(),
+            '<p>access by key: &lt;Folder at test_folder_1_&gt;</p>')
+        self.app.test_folder_1_.__roles__ = ['Manager']
+        self.assertRaises(Unauthorized, pt.pt_render)
+
+    def test_cook_zope2_page_templates_bad_key_unicode(self):
+        from Products.PageTemplates.ZopePageTemplate import ZopePageTemplate
+        pt = ZopePageTemplate('mytemplate', BAD_KEY_UNICODE)
+        hack_pt(pt, self.app)
+        self.assertEqual(
+            pt.pt_render(),
+            '<p>access by key: &lt;Folder at test_folder_1_&gt;</p>')
+        self.app.test_folder_1_.__roles__ = ['Manager']
+        self.assertRaises(Unauthorized, pt.pt_render)
+
+    def test_cook_zope2_page_templates_bad_item_str(self):
+        from Products.PageTemplates.ZopePageTemplate import ZopePageTemplate
+        self.app.testlist = [self.app.test_folder_1_]
+        pt = ZopePageTemplate('mytemplate', BAD_ITEM_STR)
+        hack_pt(pt, self.app.testlist)
+        self.assertEqual(
+            pt.pt_render(),
+            '<p>access by item: &lt;Folder at test_folder_1_&gt;</p>')
+        self.app.test_folder_1_.__roles__ = ['Manager']
+        self.assertRaises(Unauthorized, pt.pt_render)
+
+    def test_cook_zope2_page_templates_bad_item_unicode(self):
+        from Products.PageTemplates.ZopePageTemplate import ZopePageTemplate
+        self.app.testlist = [self.app.test_folder_1_]
+        pt = ZopePageTemplate('mytemplate', BAD_ITEM_UNICODE)
+        hack_pt(pt, self.app.testlist)
+        self.assertEqual(
+            pt.pt_render(),
+            '<p>access by item: &lt;Folder at test_folder_1_&gt;</p>')
+        self.app.test_folder_1_.__roles__ = ['Manager']
+        self.assertRaises(Unauthorized, pt.pt_render)
+
+    def assert_is_checked_via_security_manager(self, pt_content):
+        from Products.PageTemplates.ZopePageTemplate import ZopePageTemplate
+        from AccessControl.SecurityManager import setSecurityPolicy
+        from AccessControl.SecurityManagement import noSecurityManager
+        from AccessControl.SecurityManagement import getSecurityManager
+
+        pt = ZopePageTemplate('mytemplate', pt_content)
+        noSecurityManager()
+        old_security_policy = setSecurityPolicy(UnauthorizedSecurityPolicy())
+        try:
+            hack_pt(pt, context=self.app)
+            self.assertRaises(Unauthorized, pt.pt_render)
+        finally:
+            setSecurityPolicy(old_security_policy)
+
+    def test_getattr_access_is_checked_via_security_manager(self):
+        self.assert_is_checked_via_security_manager(
+            """<p tal:content="python:'{0.acl_users}'.format(context)" />""")
+
+    def test_getitem_access_is_checked_via_security_manager(self):
+        self.assert_is_checked_via_security_manager(
+            """<p tal:content="python:'{c[acl_users]}'.format(c=context)" />"""
+        )
+
+    def test_key_access_is_checked_via_security_manager(self):
+        self.assert_is_checked_via_security_manager(
+            """<p tal:content="python:'{c[0]}'.format(c=[context])" />"""
+        )
diff --git a/src/Zope2/App/tests/using_format_zope3_page_template.pt b/src/Zope2/App/tests/using_format_zope3_page_template.pt
@@ -1,2 +1,6 @@
 <p tal:content="python:'class of {0} is {0.__class__}'.format(context).lower()" />
 <p tal:content="python:u'class of {0} is {0.__class__}'.format(context).upper()" />
+<p tal:content="python:'{0} has foo={0[foo]}'.format({'foo': context.test_folder_1_})" />
+<p tal:content="python:u'{0} has foo={0[foo]}'.format({'foo': context.test_folder_1_})" />
+<p tal:content="python:'{0} has first item {0[0]}'.format([context.test_folder_1_])" />
+<p tal:content="python:u'{0} has first item {0[0]}'.format([context.test_folder_1_])" />