In str.format, check the security for attributes that are accessed.

Part of PloneHotfix20170117.

CHANGES.txt

@@ -4,6 +4,9 @@ Changelog
 2.13.15 (unreleased)
 --------------------
 
+- In ``str.format``, check the security for attributes that are accessed.
+  Part of PloneHotfix20170117.  [maurits]
+
 - Added ``override_container`` context manager.  Used this in tests to
   make them pass when the standard permissive security assertions for
   strings has been changed.  [maurits]

src/AccessControl/__init__.py

@@ -29,6 +29,8 @@
 from AccessControl.unauthorized import Unauthorized
 from AccessControl.ZopeGuards import full_write_guard
 from AccessControl.ZopeGuards import safe_builtins
+from AccessControl.safe_formatter import safe_format
+
 
 ModuleSecurityInfo('AccessControl').declarePublic('getSecurityManager')
 
@@ -38,3 +40,15 @@
     ModuleSecurityInfo(name).setDefaultAccess('allow')
 
 ModuleSecurityInfo('DateTime').declarePublic('DateTime')
+
+# We want to allow all methods on string type except "format".
+# That one needs special handling to avoid access to attributes.
+# from Products.PageTemplates.safe_formatter import safe_format
+rules = dict([(m, True) for m in dir(str) if not m.startswith('_')])
+rules['format'] = safe_format
+allow_type(str, rules)
+
+# Same for unicode instead of str.
+rules = dict([(m, True) for m in dir(unicode) if not m.startswith('_')])
+rules['format'] = safe_format
+allow_type(unicode, rules)

src/AccessControl/safe_formatter.py

@@ -0,0 +1,75 @@
+from AccessControl.ZopeGuards import guarded_getattr
+from collections import Mapping
+
+import string
+
+
+class _MagicFormatMapping(Mapping):
+    """
+    Pulled from Jinja2
+
+    This class implements a dummy wrapper to fix a bug in the Python
+    standard library for string formatting.
+
+    See http://bugs.python.org/issue13598 for information about why
+    this is necessary.
+    """
+
+    def __init__(self, args, kwargs):
+        self._args = args
+        self._kwargs = kwargs
+        self._last_index = 0
+
+    def __getitem__(self, key):
+        if key == '':
+            idx = self._last_index
+            self._last_index += 1
+            try:
+                return self._args[idx]
+            except LookupError:
+                pass
+            key = str(idx)
+        return self._kwargs[key]
+
+    def __iter__(self):
+        return iter(self._kwargs)
+
+    def __len__(self):
+        return len(self._kwargs)
+
+
+class SafeFormatter(string.Formatter):
+
+    def __init__(self, value):
+        self.value = value
+        super(SafeFormatter, self).__init__()
+
+    def get_field(self, field_name, args, kwargs):
+        """
+        Here we're overriding so we can use guarded_getattr instead of
+        regular getattr
+        """
+        first, rest = field_name._formatter_field_name_split()
+
+        obj = self.get_value(first, args, kwargs)
+
+        # loop through the rest of the field_name, doing
+        #  getattr or getitem as needed
+        for is_attr, i in rest:
+            if is_attr:
+                obj = guarded_getattr(obj, i)
+            else:
+                obj = obj[i]
+
+        return obj, first
+
+    def safe_format(self, *args, **kwargs):
+        kwargs = _MagicFormatMapping(args, kwargs)
+        return self.vformat(self.value, args, kwargs)
+
+
+def safe_format(inst, method):
+    """
+    Use our SafeFormatter that uses guarded_getattr for attribute access
+    """
+    return SafeFormatter(inst).safe_format

src/AccessControl/tests/normal_zope3_page_template.pt

@@ -0,0 +1,2 @@
+<p tal:content="python:('%s' % context).lower()" />
+<p tal:content="python:(u'%s' % context).upper()" />

src/AccessControl/tests/test_formatter.py

@@ -0,0 +1,119 @@
+from Testing.ZopeTestCase import FunctionalTestCase
+from zExceptions import Unauthorized
+
+import unittest
+
+
+BAD_STR = """
+<p tal:content="python:'class of {0} is {0.__class__}'.format(context)" />
+"""
+BAD_UNICODE = """
+<p tal:content="python:u'class of {0} is {0.__class__}'.format(context)" />
+"""
+GOOD_STR = '<p tal:content="python:(\'%s\' % context).lower()" />'
+GOOD_UNICODE = '<p tal:content="python:(\'%s\' % context).lower()" />'
+
+
+def noop(context=None):
+    return lambda: context
+
+
+def hack_pt(pt, context=None):
+    # hacks to avoid getting error in pt_render.
+    pt.getPhysicalRoot = noop()
+    pt._getContext = noop(context)
+    pt._getContainer = noop(context)
+    pt.context = context
+
+
+class FormatterTest(unittest.TestCase):
+
+    def test_cook_zope2_page_templates_bad_str(self):
+        from Products.PageTemplates.ZopePageTemplate import ZopePageTemplate
+        pt = ZopePageTemplate('mytemplate', BAD_STR)
+        hack_pt(pt)
+        self.assertRaises(Unauthorized, pt.pt_render)
+
+    def test_cook_zope2_page_templates_bad_unicode(self):
+        from Products.PageTemplates.ZopePageTemplate import ZopePageTemplate
+        pt = ZopePageTemplate('mytemplate', BAD_UNICODE)
+        hack_pt(pt)
+        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>')
+
+    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>')
+
+    def test_positional_argument_regression(self):
+        """
+        to test http://bugs.python.org/issue13598 issue
+        """
+        from AccessControl.safe_formatter import SafeFormatter
+        try:
+            self.assertEquals(
+                SafeFormatter('{} {}').safe_format('foo', 'bar'),
+                'foo bar'
+            )
+        except ValueError:
+            # On Python 2.6 you get:
+            # ValueError: zero length field name in format
+            pass
+
+        self.assertEquals(
+            SafeFormatter('{0} {1}').safe_format('foo', 'bar'),
+            'foo bar'
+        )
+        self.assertEquals(
+            SafeFormatter('{1} {0}').safe_format('foo', 'bar'),
+            'bar foo'
+        )
+
+
+class FormatterFunctionalTest(FunctionalTestCase):
+
+    def test_access_to_private_content_not_allowed_via_any_attribute(self):
+        from Products.PageTemplates.ZopePageTemplate import ZopePageTemplate
+        # If access to _delObject would be allowed, it would still only say
+        # something like 'bound method _delObject', without actually deleting
+        # anything, because methods are not executed in str.format, but there
+        # may be @properties that give an attacker secret info.
+        pt = ZopePageTemplate('mytemplate', '''
+<p tal:content="structure python:'access {0._delObject}'.format(context)" />
+''')
+        hack_pt(pt, context=self.app)
+        self.assertRaises(Unauthorized, pt.pt_render)
+
+    # Zope 3 templates are always file system templates.  So we actually have
+    # no problems allowing str.format there.
+
+    def test_cook_zope3_page_templates_normal(self):
+        from Products.Five.browser.pagetemplatefile import ViewPageTemplateFile
+        pt = ViewPageTemplateFile('normal_zope3_page_template.pt')
+        hack_pt(pt)
+        # Need to pass a namespace.
+        namespace = {'context': self.app}
+        self.assertEqual(
+            pt.pt_render(namespace).strip(),
+            u'<p>&lt;application at &gt;</p>\n'
+            u'<p>&lt;APPLICATION AT &gt;</p>')
+
+    def test_cook_zope3_page_templates_using_format(self):
+        from Products.Five.browser.pagetemplatefile import ViewPageTemplateFile
+        pt = ViewPageTemplateFile('using_format_zope3_page_template.pt')
+        hack_pt(pt)
+        # Need to pass a namespace.
+        namespace = {'context': self.app}
+        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>")

src/AccessControl/tests/using_format_zope3_page_template.pt

@@ -0,0 +1,2 @@
+<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()" />