Skip to content

Commit

Permalink
Merge pull request from GHSA-xjw2-6jm9-rf67
Browse files Browse the repository at this point in the history
* fix information disclosure problems through the `format*` methods of the `str` class and its instances

* Suggestion from @icemac

Co-authored-by: Michael Howitz <mh@gocept.com>

* prevent access to `string.Formatter`

* fix syntax in `CHANGES.rst`

---------

Co-authored-by: Michael Howitz <mh@gocept.com>
  • Loading branch information
d-maurer and Michael Howitz committed Aug 30, 2023
1 parent 41a183d commit 4134aed
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 13 deletions.
5 changes: 5 additions & 0 deletions CHANGES.rst
Expand Up @@ -22,6 +22,11 @@ Fixes

- Forbid using some attributes providing access to restricted Python internals.

- Fix information disclosure problems through
Python's "format" functionality
(``format`` and ``format_map`` methods on ``str`` and its instances,
``string.Formatter``).


6.0 (2022-11-03)
----------------
Expand Down
6 changes: 4 additions & 2 deletions src/RestrictedPython/Guards.py
Expand Up @@ -246,9 +246,11 @@ def safer_getattr(object, name, default=None, getattr=getattr):
http://lucumr.pocoo.org/2016/12/29/careful-with-str-format/
"""
if isinstance(object, str) and name == 'format':
if name in ('format', 'format_map') and (
isinstance(object, str) or
(isinstance(object, type) and issubclass(object, str))):
raise NotImplementedError(
'Using format() on a %s is not safe.' % object.__class__.__name__)
'Using the format*() methods of `str` is not safe')
if name.startswith('_'):
raise AttributeError(
'"{name}" is an invalid attribute name because it '
Expand Down
16 changes: 15 additions & 1 deletion src/RestrictedPython/Utilities.py
Expand Up @@ -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(
f"{self.__mod.__name__}.{attr} is not safe")
return getattr(self.__mod, attr)


utility_builtins['string'] = _AttributeDelegator(string, "Formatter")
utility_builtins['math'] = math
utility_builtins['random'] = random
utility_builtins['whrandom'] = random
Expand Down
5 changes: 4 additions & 1 deletion tests/builtins/test_utilities.py
Expand Up @@ -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():
Expand Down
58 changes: 49 additions & 9 deletions tests/test_Guards.py
Expand Up @@ -160,7 +160,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:
Expand All @@ -171,17 +171,57 @@ 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 format*() methods of `str` is not safe' == str(err.value)


UNICODE_DOT_FORMAT_DENIED = """\
a = u'Hello {}'
b = a.format(u'world')
# contributed by Ward Theunisse
STRING_DOT_FORMAT_MAP_DENIED = """\
a = 'Hello {foo.__dict__}'
b = a.format_map({foo:str})
"""


def test_Guards__safer_getattr__2():
"""It prevents using the format method of a unicode.
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 format*() methods of `str` 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 format*() methods of `str` 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/
Expand All @@ -190,8 +230,8 @@ def test_Guards__safer_getattr__2():
'__builtins__': safe_builtins,
}
with pytest.raises(NotImplementedError) as err:
restricted_exec(UNICODE_DOT_FORMAT_DENIED, glb)
assert 'Using format() on a str is not safe.' == str(err.value)
restricted_exec(STR_DOT_FORMAT_MAP_DENIED, glb)
assert 'Using the format*() methods of `str` is not safe' == str(err.value)


SAFER_GETATTR_ALLOWED = """\
Expand Down
13 changes: 13 additions & 0 deletions 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():
Expand Down Expand Up @@ -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)

0 comments on commit 4134aed

Please sign in to comment.