Skip to content

Commit

Permalink
additional check for safer_getattr (#285)
Browse files Browse the repository at this point in the history
* add an additional check for potential breakout capability via Inspection Attributes Names in the provided safer_getattr method.

---------

Co-authored-by: Dieter Maurer <d-maurer@users.noreply.github.com>
Co-authored-by: Michael Howitz <icemac@gmx.net>
  • Loading branch information
3 people authored Aug 7, 2024
1 parent d0e97cd commit 57a2acb
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 0 deletions.
4 changes: 4 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ Changes

- Allow to use the package with Python 3.13 -- Caution: No security
audit has been done so far.
- Increase the safety level of ``safer_getattr`` allowing applications to use
it as ``getattr`` implementation. Such use should now follow the same policy
and give the same level of protection as direct attribute access in an
environment based on ``RestrictedPython``'s ``safe_builtints``.


7.2 (2024-08-02)
Expand Down
5 changes: 5 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,10 @@ 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
34 changes: 34 additions & 0 deletions tests/test_Guards.py
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,40 @@ def test_Guards__safer_getattr__4():
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():
"""It should not be allowed to access global builtins in Python3."""
result = compile_restricted_exec('builtins["getattr"]')
Expand Down

0 comments on commit 57a2acb

Please sign in to comment.