Skip to content

Commit

Permalink
add an additional check for potential breakout capability via Inspect…
Browse files Browse the repository at this point in the history
…ion Attributes Names in the provided safer_getattr method.
  • Loading branch information
loechel committed Aug 5, 2024
1 parent d0e97cd commit a5b40f3
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 45 deletions.
2 changes: 2 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ Changes

- Allow to use the package with Python 3.13 -- Caution: No security
audit has been done so far.
- Add an additional check for potential breakout capability via inspection attribute names
in the provided ``safer_getattr`` method that is part of the ``safer_builtins``.


7.2 (2024-08-02)
Expand Down
6 changes: 6 additions & 0 deletions src/RestrictedPython/Guards.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import builtins

from RestrictedPython._compat import IS_PY311_OR_GREATER
from RestrictedPython.transformer import INSPECT_ATTRIBUTES


safe_builtins = {}
Expand Down Expand Up @@ -253,6 +254,11 @@ def safer_getattr(object, name, default=None, getattr=getattr):
(isinstance(object, type) and issubclass(object, str))):
raise NotImplementedError(
'Using the format*() methods of `str` is not safe')
if name in INSPECT_ATTRIBUTES:
raise AttributeError(
f'"{name}" is a restricted name,'
' that is forbidden to access in RestrictedPython.',
)
if name.startswith('_'):
raise AttributeError(
'"{name}" is an invalid attribute name because it '
Expand Down
132 changes: 87 additions & 45 deletions tests/test_Guards.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,96 +15,101 @@ def _write_(x):

def test_Guards_bytes():
"""It contains bytes"""
assert restricted_eval('bytes(1)') == bytes(1)
assert restricted_eval("bytes(1)") == bytes(1)


def test_Guards_sorted():
"""It contains sorted"""
assert restricted_eval('sorted([5, 2, 8, 1])') == sorted([5, 2, 8, 1])
assert restricted_eval("sorted([5, 2, 8, 1])") == sorted([5, 2, 8, 1])


def test_Guards__safe_builtins__1():
"""It contains `slice()`."""
assert restricted_eval('slice(1)') == slice(1)
assert restricted_eval("slice(1)") == slice(1)


def test_Guards__safe_builtins__2():
"""It allows to define new classes by allowing `__build_class__`.
"""
"""It allows to define new classes by allowing `__build_class__`."""

class_can_be_defined_code = '''
class_can_be_defined_code = """
class MyClass:
value = None
def display(self):
return str(self.value)
ob1 = MyClass()
ob1.value = 2411
result = ob1.display()'''
result = ob1.display()"""

restricted_globals = dict(
result=None,
__name__='restricted_module',
__name__="restricted_module",
__metaclass__=type,
_write_=_write_,
_getattr_=getattr)
_getattr_=getattr,
)

restricted_exec(class_can_be_defined_code, restricted_globals)
assert restricted_globals['result'] == '2411'
assert restricted_globals["result"] == "2411"


def test_Guards__guarded_setattr__1():
"""It allows use setattr and delattr when _guarded_writes is True.
"""
"""It allows use setattr and delattr when _guarded_writes is True."""

class MyObjectD:
value = None
_guarded_writes = 1

setattr_code = '''
setattr_code = """
my_object_d = MyObjectD()
setattr(my_object_d, 'value', 9999)'''
setattr(my_object_d, 'value', 9999)"""

delattr_code = "delattr(my_object_d, 'value')"

restricted_globals = dict(
__builtins__=safe_builtins,
MyObjectD=MyObjectD,
my_object_d=None,
__name__='restricted_module',
__name__="restricted_module",
__metaclass__=type,
_write_=_write_,
_getattr_=getattr,)
_getattr_=getattr,
)

restricted_exec(setattr_code, restricted_globals)
assert 9999 == restricted_globals['my_object_d'].value
assert 9999 == restricted_globals["my_object_d"].value

restricted_exec(delattr_code, restricted_globals)
assert None is restricted_globals['my_object_d'].value
assert None is restricted_globals["my_object_d"].value


def test_Guards__write_wrapper__1():
"""It wraps the value attribute when it is not
marked with _guarded_writes."""

class ObjWithoutGuardedWrites:
my_attr = None

setattr_without_guarded_writes_code = '''
setattr_without_guarded_writes_code = """
my_ob = ObjWithoutGuardedWrites()
setattr(my_ob, 'my_attr', 'bar')'''
setattr(my_ob, 'my_attr', 'bar')"""

restricted_globals = dict(
__builtins__=safe_builtins,
ObjWithoutGuardedWrites=ObjWithoutGuardedWrites,
my_attr=None,
__name__='restricted_module',
__name__="restricted_module",
__metaclass__=type,
_write_=_write_,
_getattr_=getattr,)
_getattr_=getattr,
)

with pytest.raises(TypeError) as excinfo:
restricted_exec(
setattr_without_guarded_writes_code, restricted_globals)
assert 'attribute-less object (assign or del)' in str(excinfo.value)
setattr_without_guarded_writes_code,
restricted_globals,
)
assert "attribute-less object (assign or del)" in str(excinfo.value)


def test_Guards__write_wrapper__2():
Expand All @@ -116,23 +121,26 @@ class ObjWithGuardedSetattr:
def __guarded_setattr__(self, key, value):
setattr(self, key, value)

set_attribute_using_guarded_setattr_code = '''
set_attribute_using_guarded_setattr_code = """
myobj_with_guarded_setattr = ObjWithGuardedSetattr()
setattr(myobj_with_guarded_setattr, 'my_attr', 'bar')
'''
"""

restricted_globals = dict(
__builtins__=safe_builtins,
ObjWithGuardedSetattr=ObjWithGuardedSetattr,
myobj_with_guarded_setattr=None,
__name__='restricted_module',
__name__="restricted_module",
__metaclass__=type,
_write_=_write_,
_getattr_=getattr,)
_getattr_=getattr,
)

restricted_exec(
set_attribute_using_guarded_setattr_code, restricted_globals)
assert restricted_globals['myobj_with_guarded_setattr'].my_attr == 'bar'
set_attribute_using_guarded_setattr_code,
restricted_globals,
)
assert restricted_globals["myobj_with_guarded_setattr"].my_attr == "bar"


def test_Guards__guarded_unpack_sequence__1(mocker):
Expand All @@ -144,13 +152,13 @@ def test_Guards__guarded_unpack_sequence__1(mocker):
_getiter_ = mocker.stub()
_getiter_.side_effect = lambda it: it
glb = {
'_getiter_': _getiter_,
'_unpack_sequence_': guarded_unpack_sequence,
"_getiter_": _getiter_,
"_unpack_sequence_": guarded_unpack_sequence,
}

with pytest.raises(ValueError) as excinfo:
restricted_exec(src, glb)
assert 'values to unpack' in str(excinfo.value)
assert "values to unpack" in str(excinfo.value)
assert _getiter_.call_count == 1


Expand All @@ -167,11 +175,11 @@ def test_Guards__safer_getattr__1a():
http://lucumr.pocoo.org/2016/12/29/careful-with-str-format/
"""
glb = {
'__builtins__': safe_builtins,
"__builtins__": safe_builtins,
}
with pytest.raises(NotImplementedError) as err:
restricted_exec(STRING_DOT_FORMAT_DENIED, glb)
assert 'Using the format*() methods of `str` is not safe' == str(err.value)
assert "Using the format*() methods of `str` is not safe" == str(err.value)


# contributed by Ward Theunisse
Expand All @@ -188,11 +196,11 @@ def test_Guards__safer_getattr__1b():
http://lucumr.pocoo.org/2016/12/29/careful-with-str-format/
"""
glb = {
'__builtins__': safe_builtins,
"__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)
assert "Using the format*() methods of `str` is not safe" == str(err.value)


# contributed by Abhishek Govindarasu
Expand All @@ -208,11 +216,11 @@ def test_Guards__safer_getattr__1c():
http://lucumr.pocoo.org/2016/12/29/careful-with-str-format/
"""
glb = {
'__builtins__': safe_builtins,
"__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)
assert "Using the format*() methods of `str` is not safe" == str(err.value)


STR_DOT_FORMAT_MAP_DENIED = """\
Expand All @@ -227,11 +235,11 @@ def test_Guards__safer_getattr__1d():
http://lucumr.pocoo.org/2016/12/29/careful-with-str-format/
"""
glb = {
'__builtins__': safe_builtins,
"__builtins__": safe_builtins,
}
with pytest.raises(NotImplementedError) as err:
restricted_exec(STR_DOT_FORMAT_MAP_DENIED, glb)
assert 'Using the format*() methods of `str` is not safe' == str(err.value)
assert "Using the format*() methods of `str` is not safe" == str(err.value)


SAFER_GETATTR_ALLOWED = """\
Expand All @@ -256,7 +264,7 @@ def test_Guards__safer_getattr__3():
result=None,
)
restricted_exec(SAFER_GETATTR_ALLOWED, restricted_globals)
assert restricted_globals['result'] == 2
assert restricted_globals["result"] == 2


SAFER_GETATTR_BREAKOUT = """\
Expand Down Expand Up @@ -292,7 +300,41 @@ def test_Guards__safer_getattr__4():

with pytest.raises(TypeError) as err:
restricted_exec(SAFER_GETATTR_BREAKOUT, restricted_globals)
assert 'type(name) must be str' == str(err.value)
assert "type(name) must be str" == str(err.value)


SAFER_GETATTR_BREAKOUT2 = """\
g = None
leak = None
def test():
global g, leak
leak = getattr(getattr(getattr(g, "gi_frame"), "f_back"), "f_back")
yield leak
g = test()
g.send(None)
os = getattr(leak, "f_builtins").get('__import__')('os')
result = os.getgid()
"""


def test_Guards__safer_getattr__5():
restricted_globals = dict(
__builtins__=safe_builtins,
__name__=None,
__metaclass__=type,
# _write_=_write_,
getattr=safer_getattr,
result=None,
)

# restricted_exec(SAFER_GETATTR_BREAKOUT2, restricted_globals)
# assert restricted_globals['result'] == 20
with pytest.raises(AttributeError) as err:
restricted_exec(SAFER_GETATTR_BREAKOUT2, restricted_globals)
assert (
'"gi_frame" is a restricted name, '
"that is forbidden to access in RestrictedPython."
) == str(err.value)


def test_call_py3_builtins():
Expand All @@ -313,9 +355,9 @@ def test_safer_getattr__underscore_name():
assert result.errors == ()
assert result.warnings == []
glb = safe_globals.copy()
glb['getattr'] = safer_getattr
glb["getattr"] = safer_getattr
with pytest.raises(AttributeError) as err:
exec(result.code, glb, {})
assert (
'"__class__" is an invalid attribute name because it starts with "_"'
== str(err.value))
) == str(err.value)

0 comments on commit a5b40f3

Please sign in to comment.