Merge pull request from GHSA-xjw2-6jm9-rf67

* fix information disclosure problems through the `format*` methods of the `str` class and its instances * update `CHANGES.rst` * update `CHANGES.rst` * prevent access to `string.Formatter` * Python 2 does not yet support f-strings * Fix CHANGES.rst according to review Co-authored-by: Michael Howitz <[email protected]> * Fix CHANGES.rst according to review Co-authored-by: Michael Howitz <[email protected]> --------- Co-authored-by: Michael Howitz <[email protected]>
zopefoundation · Aug 30, 2023 · 66847bc · 66847bc
1 parent e170fd2
commit 66847bc
Show file tree

Hide file tree

Showing 6 changed files with 176 additions and 13 deletions.
diff --git a/CHANGES.rst b/CHANGES.rst
@@ -4,6 +4,15 @@ Changes
 5.4 (unreleased)
 ----------------
 
+Fixes
++++++
+
+- Fix information disclosure problems through
+  Python's "format" functionality
+  (``format`` and ``format_map`` methods on ``str``/``unicode`` and
+  their instances,
+  ``string.Formatter``).
+
 
 5.3 (2023-07-08)
 ----------------

diff --git a/src/RestrictedPython/Guards.py b/src/RestrictedPython/Guards.py
@@ -264,9 +264,12 @@ def safer_getattr(object, name, default=None, getattr=getattr):
     http://lucumr.pocoo.org/2016/12/29/careful-with-str-format/
 
     """
-    if isinstance(object, _compat.basestring) and name == 'format':
+    if name in ('format', 'format_map') and (
+            isinstance(object, _compat.basestring) or (
+            isinstance(object, type)
+            and issubclass(object, _compat.basestring))):
         raise NotImplementedError(
-            'Using format() on a %s is not safe.' % object.__class__.__name__)
+            'Using the string format* methods is not safe')
     if name.startswith('_'):
         raise AttributeError(
             '"{name}" is an invalid attribute name because it '

diff --git a/src/RestrictedPython/Utilities.py b/src/RestrictedPython/Utilities.py
@@ -18,7 +18,21 @@
 
 utility_builtins = {}
 
-utility_builtins['string'] = string
+
+class _AttributeDelegator:
+    def __init__(self, mod, *excludes):
+        """delegate attribute lookups outside *excludes* to module *mod*."""
+        self.__mod = mod
+        self.__excludes = excludes
+
+    def __getattr__(self, attr):
+        if attr in self.__excludes:
+            raise NotImplementedError(
+                "{}.{} is not safe".format(self.__mod.__name__, attr))
+        return getattr(self.__mod, attr)
+
+
+utility_builtins['string'] = _AttributeDelegator(string, "Formatter")
 utility_builtins['math'] = math
 utility_builtins['random'] = random
 utility_builtins['whrandom'] = random

diff --git a/tests/builtins/test_utilities.py b/tests/builtins/test_utilities.py
@@ -5,7 +5,10 @@
 
 def test_string_in_utility_builtins():
     from RestrictedPython.Utilities import utility_builtins
-    assert utility_builtins['string'] is string
+
+    # we no longer provide access to ``string`` itself, only to
+    # a restricted view of it
+    assert utility_builtins['string'].__name__ == string.__name__
 
 
 def test_math_in_utility_builtins():

diff --git a/tests/test_Guards.py b/tests/test_Guards.py
@@ -162,7 +162,7 @@ def test_Guards__guarded_unpack_sequence__1(mocker):
 """
 
 
-def test_Guards__safer_getattr__1():
+def test_Guards__safer_getattr__1a():
     """It prevents using the format method of a string.
 
     format() is considered harmful:
@@ -173,16 +173,120 @@ def test_Guards__safer_getattr__1():
     }
     with pytest.raises(NotImplementedError) as err:
         restricted_exec(STRING_DOT_FORMAT_DENIED, glb)
-    assert 'Using format() on a str is not safe.' == str(err.value)
+    assert 'Using the string format* methods is not safe' == str(err.value)
 
 
-UNICODE_DOT_FORMAT_DENIED = """\
+# contributed by Ward Theunisse
+STRING_DOT_FORMAT_MAP_DENIED = """\
+a = 'Hello {foo.__dict__}'
+b = a.format_map({foo:str})
+"""
+
+
+def test_Guards__safer_getattr__1b():
+    """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:
+        restricted_exec(STRING_DOT_FORMAT_MAP_DENIED, glb)
+    assert 'Using the string format* methods is not safe' == str(err.value)
+
+
+# contributed by Abhishek Govindarasu
+STR_DOT_FORMAT_DENIED = """\
+str.format('{0.__class__.__mro__[1]}', int)
+"""
+
+
+def test_Guards__safer_getattr__1c():
+    """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:
+        restricted_exec(STR_DOT_FORMAT_DENIED, glb)
+    assert 'Using the string format* methods is not safe' == str(err.value)
+
+
+STR_DOT_FORMAT_MAP_DENIED = """\
+str.format_map('Hello {foo.__dict__}', {'foo':str})
+"""
+
+
+def test_Guards__safer_getattr__1d():
+    """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:
+        restricted_exec(STR_DOT_FORMAT_MAP_DENIED, glb)
+    assert 'Using the string format* methods is not safe' == str(err.value)
+
+
+USTRING_DOT_FORMAT_DENIED = """\
 a = u'Hello {}'
-b = a.format(u'world')
+b = a.format('world')
 """
 
 
-def test_Guards__safer_getattr__2():
+@pytest.mark.skipif(IS_PY3, reason="Python 3 lacks unicode")
+def test_Guards__safer_getattr__2a():
+    """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:
+        restricted_exec(USTRING_DOT_FORMAT_DENIED, glb)
+    assert 'Using the string format* methods is not safe' == str(err.value)
+
+
+# contributed by Ward Theunisse
+USTRING_DOT_FORMAT_MAP_DENIED = """\
+a = u'Hello {foo.__dict__}'
+b = a.format_map({foo:str})
+"""
+
+
+@pytest.mark.skipif(IS_PY3, reason="Python 3 lacks unicode")
+def test_Guards__safer_getattr__2b():
+    """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:
+        restricted_exec(USTRING_DOT_FORMAT_MAP_DENIED, glb)
+    assert 'Using the string format* methods is not safe' == str(err.value)
+
+
+# contributed by Abhishek Govindarasu
+UNICODE_DOT_FORMAT_DENIED = """\
+unicode.format(u'{0.__class__.__mro__[1]}', int)
+"""
+
+
+@pytest.mark.skipif(IS_PY3, reason="Python 3 lacks unicode")
+def test_Guards__safer_getattr__2c():
     """It prevents using the format method of a unicode.
 
     format() is considered harmful:
@@ -193,10 +297,27 @@ def test_Guards__safer_getattr__2():
     }
     with pytest.raises(NotImplementedError) as err:
         restricted_exec(UNICODE_DOT_FORMAT_DENIED, glb)
-    if IS_PY2:  # pragma: PY2
-        assert 'Using format() on a unicode is not safe.' == str(err.value)
-    else:  # pragma: PY3
-        assert 'Using format() on a str is not safe.' == str(err.value)
+    assert 'Using the string format* methods is not safe' == str(err.value)
+
+
+UNICODE_DOT_FORMAT_MAP_DENIED = """\
+unicode.format_map(u'Hello {foo.__dict__}', {'foo':str})
+"""
+
+
+@pytest.mark.skipif(IS_PY3, reason="Python 3 lacks unicode")
+def test_Guards__safer_getattr__2d():
+    """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:
+        restricted_exec(UNICODE_DOT_FORMAT_MAP_DENIED, glb)
+    assert 'Using the string format* methods is not safe' == str(err.value)
 
 
 SAFER_GETATTR_ALLOWED = """\

diff --git a/tests/test_Utilities.py b/tests/test_Utilities.py
@@ -1,5 +1,8 @@
+import pytest
+
 from RestrictedPython.Utilities import reorder
 from RestrictedPython.Utilities import test
+from RestrictedPython.Utilities import utility_builtins
 
 
 def test_Utilities__test_1():
@@ -30,3 +33,13 @@ def test_Utilities__reorder_1():
     _with = [('k2', 'v2'), ('k3', 'v3')]
     without = [('k2', 'v2'), ('k4', 'v4')]
     assert reorder(s, _with, without) == [('k3', 'v3')]
+
+
+def test_Utilities_string_Formatter():
+    """Access to ``string.Formatter`` is denied."""
+    string = utility_builtins["string"]
+    # access successful in principle
+    assert string.ascii_lowercase == 'abcdefghijklmnopqrstuvwxyz'
+    with pytest.raises(NotImplementedError) as exc:
+        string.Formatter
+    assert 'string.Formatter is not safe' == str(exc.value)