In str.format, check the security for keys and items that are accesse…

…d. (#50)

Part of PloneHotfix20171128.

CHANGES.txt

@@ -4,6 +4,9 @@ Changelog
 2.13.16 (unreleased)
 --------------------
 
+- In ``str.format``, check the security for keys and items that are accessed.
+  Part of PloneHotfix20171128.  [maurits]
+
 - set explicit PyPI index, the old ``zc.buildout`` defaults no longer work
 
 - add ``tox`` testing configuration

src/AccessControl/safe_formatter.py

@@ -1,12 +1,11 @@
-from AccessControl.ZopeGuards import guarded_getattr
+from AccessControl.ZopeGuards import guarded_getattr, guarded_getitem
 from collections import Mapping
 
 import string
 
 
 class _MagicFormatMapping(Mapping):
-    """
-    Pulled from Jinja2
+    """Pulled from Jinja2.
 
     This class implements a dummy wrapper to fix a bug in the Python
     standard library for string formatting.
@@ -39,16 +38,14 @@ def __len__(self):
 
 
 class SafeFormatter(string.Formatter):
+    """Formatter using guarded access."""
 
     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
-        """
+        """Get the field value using guarded methods."""
         first, rest = field_name._formatter_field_name_split()
 
         obj = self.get_value(first, args, kwargs)
@@ -59,17 +56,16 @@ def get_field(self, field_name, args, kwargs):
             if is_attr:
                 obj = guarded_getattr(obj, i)
             else:
-                obj = obj[i]
+                obj = guarded_getitem(obj, i)
 
         return obj, first
 
     def safe_format(self, *args, **kwargs):
+        """Safe variant of `format` method."""
         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
-    """
+    """Use our SafeFormatter that uses guarded_getattr and guarded_getitem."""
     return SafeFormatter(inst).safe_format

src/AccessControl/tests/test_formatter.py

@@ -1,7 +1,55 @@
 from zExceptions import Unauthorized
+from persistent import Persistent
+
 import unittest
 
 
+SliceType = type(slice(0))
+
+
+class Item(Persistent):
+
+    def __init__(self, id, private=False):
+        self.id = id
+        if private:
+            self.__roles__ = ['Manager']
+        else:
+            self.__roles__ = ['Anonymous']
+
+    def __repr__(self):
+        return '<Item {0}>'.format(self.id)
+
+
+class Folder(Persistent):
+
+    def __init__(self, id):
+        self.id = id
+        self.item_dict = {}
+        self.item_list = []
+
+    def addItem(self, name, private=False):
+        item = Item(name, private)
+        # add as attribute for testing attribute access:
+        setattr(self, name, item)
+        # add in dict for testing named item access:
+        self.item_dict[name] = item
+        # add in list for testing numeric item access:
+        self.item_list.append(item)
+
+    def __getitem__(self, key):
+        # key can be a slice.
+        if isinstance(key, SliceType):
+            return self.item_list[key]
+        # Is this numeric (integer) access or string access?
+        # We could use isinstance(key, (int, long)), but long
+        # is not defined in Python 3.
+        try:
+            key = int(key)
+        except (ValueError, TypeError):
+            return self.item_dict[key]
+        return self.item_list[key]
+
+
 class FormatterTest(unittest.TestCase):
     """Test our safe formatter.
 
@@ -12,6 +60,14 @@ class FormatterTest(unittest.TestCase):
     itself.
     """
 
+    def _create_folder_with_mixed_contents(self):
+        """Create a folder with mixed public and private contents."""
+        folder = Folder('folder')
+        folder.addItem('public1')
+        folder.addItem('private', private=True)
+        folder.addItem('public2')
+        return folder
+
     def test_positional_argument_regression(self):
         """
         to test http://bugs.python.org/issue13598 issue
@@ -36,14 +92,129 @@ def test_positional_argument_regression(self):
             'bar foo'
         )
 
-    def test_prevents_bad_string_formatting(self):
+    def test_prevents_bad_string_formatting_attribute(self):
         from AccessControl.safe_formatter import SafeFormatter
+        # Accessing basic Python attributes on a basic Python type is fine.
+        self.assertTrue(
+            SafeFormatter('{0.upper}').safe_format('foo').startswith(
+            '<built-in method upper'))
+        # unless the name is protected
+        self.assertRaises(Unauthorized,
+                          SafeFormatter('{0.__class__}').safe_format, 'foo')
+        # But for non-basic items or non-basic lists, we want run checks.
+        folder = self._create_folder_with_mixed_contents()
+        # We can get the public items just fine:
+        self.assertEqual(SafeFormatter('{0.public1}').safe_format(folder),
+                         '<Item public1>')
+        self.assertEqual(SafeFormatter('{0.public2}').safe_format(folder),
+                         '<Item public2>')
+        # But not the private item:
+        self.assertRaises(Unauthorized,
+                          SafeFormatter('{0.private}').safe_format,
+                          folder)
+
+    def test_prevents_bad_unicode_formatting_attribute(self):
+        from AccessControl.safe_formatter import SafeFormatter
+        # Accessing basic Python attributes on a basic Python type is fine.
+        self.assertTrue(
+            SafeFormatter(u'{0.upper}').safe_format('foo').startswith(
+            '<built-in method upper'))
+        # unless the name is protected
+        self.assertRaises(Unauthorized,
+                          SafeFormatter(u'{0.__class__}').safe_format, 'foo')
+        # But for non-basic items or non-basic lists, we want run checks.
+        folder = self._create_folder_with_mixed_contents()
+        # We can get the public items just fine:
+        self.assertEqual(SafeFormatter(u'{0.public1}').safe_format(folder),
+                         '<Item public1>')
+        self.assertEqual(SafeFormatter(u'{0.public2}').safe_format(folder),
+                         '<Item public2>')
+        # But not the private item:
+        self.assertRaises(Unauthorized,
+                          SafeFormatter(u'{0.private}').safe_format,
+                          folder)
+
+    def test_prevents_bad_string_formatting_item(self):
+        from AccessControl.safe_formatter import SafeFormatter
+        # Accessing basic Python types in a basic Python dict is fine.
+        foo = {'bar': 'Can you see me?'}
+        self.assertEqual(SafeFormatter('{0[bar]}').safe_format(foo),
+                         'Can you see me?')
+        # But for non-basic items or non-basic lists, we want run checks.
+        folder = self._create_folder_with_mixed_contents()
+        # We can get the public items just fine:
+        self.assertEqual(SafeFormatter('{0[public1]}').safe_format(folder),
+                         '<Item public1>')
+        self.assertEqual(SafeFormatter('{0[public2]}').safe_format(folder),
+                         '<Item public2>')
+        # But not the private item:
+        self.assertRaises(Unauthorized,
+                          SafeFormatter('{0[private]}').safe_format,
+                          folder)
+
+    def test_prevents_bad_unicode_formatting_item(self):
+        from AccessControl.safe_formatter import SafeFormatter
+        # Accessing basic Python types in a basic Python dict is fine.
+        foo = {'bar': 'Can you see me?'}
+        self.assertEqual(SafeFormatter(u'{0[bar]}').safe_format(foo),
+                         'Can you see me?')
+        # But for non-basic items or non-basic lists, we want run checks.
+        folder = self._create_folder_with_mixed_contents()
+        # We can get the public items just fine:
+        self.assertEqual(SafeFormatter(u'{0[public1]}').safe_format(folder),
+                         '<Item public1>')
+        self.assertEqual(SafeFormatter(u'{0[public2]}').safe_format(folder),
+                         '<Item public2>')
+        # But not the private item:
+        self.assertRaises(Unauthorized,
+                          SafeFormatter(u'{0[private]}').safe_format,
+                          folder)
+
+    def test_prevents_bad_string_formatting_key(self):
+        from AccessControl.safe_formatter import SafeFormatter
+        from AccessControl.ZopeGuards import guarded_getitem
+        from persistent.list import PersistentList
+        # Accessing basic Python types in a basic Python list is fine.
+        foo = list(['bar'])
+        self.assertEqual(SafeFormatter('{0[0]}').safe_format(foo),
+                         'bar')
+        self.assertEqual(guarded_getitem(foo, 0), 'bar')
+        # For basic Python types in a non-basic list, we guard the access.
+        foo = PersistentList(foo)
+        self.assertRaises(Unauthorized, guarded_getitem, foo, 0)
+        self.assertRaises(Unauthorized,
+                          SafeFormatter('{0[0]}').safe_format, foo)
+        # though we could allow access if we want:
+        foo.__allow_access_to_unprotected_subobjects__ = 1
+        self.assertEqual(guarded_getitem(foo, 0), 'bar')
+        self.assertEqual(SafeFormatter('{0[0]}').safe_format(foo),
+                         'bar')
+        # For non-basic items we want run checks too.
+        folder = self._create_folder_with_mixed_contents()
+        # We can get the public items just fine:
+        self.assertEqual(SafeFormatter('{0[0]}').safe_format(folder),
+                         '<Item public1>')
+        self.assertEqual(SafeFormatter('{0[2]}').safe_format(folder),
+                         '<Item public2>')
+        # But not the private item:
         self.assertRaises(Unauthorized,
-                          SafeFormatter('{0.__class__}').safe_format,
-                          'foo')
+                          SafeFormatter('{0[1]}').safe_format,
+                          folder)
 
-    def test_prevents_bad_unicode_formatting(self):
+    def test_prevents_bad_unicode_formatting_key(self):
         from AccessControl.safe_formatter import SafeFormatter
+        # Accessing basic Python types in a basic Python list is fine.
+        foo = list(['bar'])
+        self.assertEqual(SafeFormatter('{0[0]}').safe_format(foo),
+                         u'bar')
+        # But for non-basic items or non-basic lists, we want run checks.
+        folder = self._create_folder_with_mixed_contents()
+        # We can get the public items just fine:
+        self.assertEqual(SafeFormatter(u'{0[0]}').safe_format(folder),
+                         '<Item public1>')
+        self.assertEqual(SafeFormatter(u'{0[2]}').safe_format(folder),
+                         '<Item public2>')
+        # But not the private item:
         self.assertRaises(Unauthorized,
-                          SafeFormatter(u'{0.__class__}').safe_format,
-                          u'foo')
+                          SafeFormatter(u'{0[1]}').safe_format,
+                          folder)