Security issue: Ship with a default implementation for _getattr_ (#83)

* Security issue: Ships with a default implementation for ``_getattr_``

It prevents from using the ``format()`` method on str/unicode as it is not
safe, see: http://lucumr.pocoo.org/2016/12/29/careful-with-str-format/

docs/CHANGES.rst

@@ -4,6 +4,16 @@ Changes
 4.0a4 (unreleased)
 ------------------
 
+- Security issue: RestrictedPython now ships with a default implementation for
+  ``_getattr_`` which prevents from using the ``format()`` method on
+  str/unicode as it is not safe, see:
+  http://lucumr.pocoo.org/2016/12/29/careful-with-str-format/
+
+  **Caution:** If you do not already have secured the access to this
+  ``format()`` method in your ``_getattr_`` implementation use
+  ``RestrictedPython.Guards.safer_getattr()`` in your implementation to
+  benefit from this fix.
+
 - Remove ``__len__`` method in ``.Guards._write_wrapper`` because it is no
   longer reachable by code using the wrapper.
 

src/RestrictedPython/Guards.py

@@ -15,10 +15,10 @@
 # AccessControl.ZopeGuards contains a large set of wrappers for builtins.
 # DocumentTemplate.DT_UTil contains a few.
 
-from ._compat import IS_PY2
+from RestrictedPython import _compat
 
 
-if IS_PY2:
+if _compat.IS_PY2:
     import __builtin__ as builtins
 else:
     # Do not attempt to use this package on Python2.7 as there
@@ -107,7 +107,7 @@
     'ZeroDivisionError',
 ]
 
-if IS_PY2:
+if _compat.IS_PY2:
     _safe_names.extend([
         'basestring',
         'cmp',
@@ -256,6 +256,22 @@ def guarded_delattr(object, name):
 safe_builtins['delattr'] = guarded_delattr
 
 
+def safer_getattr(object, name, getattr=getattr):
+    """Getattr implementation which prevents using format on string objects.
+
+    format() is considered harmful:
+    http://lucumr.pocoo.org/2016/12/29/careful-with-str-format/
+
+    """
+    if isinstance(object, _compat.basestring) and name == 'format':
+        raise NotImplementedError(
+            'Using format() on a %s is not safe.' % object.__class__.__name__)
+    return getattr(object, name)
+
+
+safe_builtins['_getattr_'] = safer_getattr
+
+
 def guarded_iter_unpack_sequence(it, spec, _getiter_):
     """Protect sequence unpacking of targets in a 'for loop'.
 

src/RestrictedPython/README.rst

@@ -55,7 +55,9 @@ Specifically:
    takes two arguments.  The first is the base object to be accessed,
    while the second is the attribute name or item index that will be
    read.  The guard function should return the attribute or subitem,
-   or raise an exception.
+   or raise an exception. RestrictedPython ships with a default implementation
+   for ``_getattr_`` which prevents from using the format method of
+   strings as it is considered harmful.
 
 4. ``__import__`` is the normal Python import hook, and should be used
    to control access to Python packages and modules.

src/RestrictedPython/_compat.py

@@ -6,3 +6,8 @@
 IS_PY3 = _version.major == 3
 IS_PY34_OR_GREATER = _version.major == 3 and _version.minor >= 4
 IS_PY35_OR_GREATER = _version.major == 3 and _version.minor >= 5
+
+if IS_PY2:
+    basestring = basestring
+else:
+    basestring = str

tests/test_Guards.py

@@ -1,3 +1,4 @@
+from RestrictedPython._compat import IS_PY2
 from RestrictedPython.Guards import guarded_unpack_sequence
 from RestrictedPython.Guards import safe_builtins
 from tests import e_eval
@@ -141,3 +142,48 @@ def test_Guards__guarded_unpack_sequence__1(e_exec, mocker):
         e_exec(src, glb)
     assert 'values to unpack' in str(excinfo.value)
     assert _getiter_.call_count == 1
+
+
+STRING_DOT_FORMAT_DENIED = """\
+a = 'Hello {}'
+b = a.format('world')
+"""
+
+
+@pytest.mark.parametrize(*e_exec)
+def test_Guards__safer_getattr__1(e_exec):
+    """It prevents using the format method of a string.
+
+    format() is considered harmful:
+    http://lucumr.pocoo.org/2016/12/29/careful-with-str-format/
+    """
+    glb = {
+        '__builtins__': safe_builtins,
+    }
+    with pytest.raises(NotImplementedError) as err:
+        e_exec(STRING_DOT_FORMAT_DENIED, glb)
+    assert 'Using format() on a str is not safe.' == str(err.value)
+
+
+UNICODE_DOT_FORMAT_DENIED = """\
+a = u'Hello {}'
+b = a.format(u'world')
+"""
+
+
+@pytest.mark.parametrize(*e_exec)
+def test_Guards__safer_getattr__2(e_exec):
+    """It prevents using the format method of a unicode.
+
+    format() is considered harmful:
+    http://lucumr.pocoo.org/2016/12/29/careful-with-str-format/
+    """
+    glb = {
+        '__builtins__': safe_builtins,
+    }
+    with pytest.raises(NotImplementedError) as err:
+        e_exec(UNICODE_DOT_FORMAT_DENIED, glb)
+    if IS_PY2:
+        assert 'Using format() on a unicode is not safe.' == str(err.value)
+    else:
+        assert 'Using format() on a str is not safe.' == str(err.value)