Repository: zopefoundation/RestrictedPython

Issue:
Provide a `safer_getattr` variant with `getattr` compatible signature
We started to advertise `safer_getattr` as a safe `getattr` implementation candidate in a typical `RestrictedPython` environment **BUT** `safer_getattr` has a different signature and semantics than `getattr`: `safer_getattr` has by default the parameter `default=None`  while `getattr` lacks this parameter by default and then raise `AttributeError` (rather than returning the default value).

I propose to provide (additionally) a variant of `safer_getattr` (say `safer_getattr_exc`) mirroring the signature and behavior of `getattr`. Application can then decide which variant they want to use for their `_getattr_`.

Discussion:

Solution patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index 10fad5c..43226ba 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -7,6 +7,13 @@ Changes
 - Allow to use the package with Python 3.13 -- Caution: No security
   audit has been done so far.
 
+- Provide new function ``RestrictedPython.Guards.safer_getattr_raise``.
+  It is similar to ``safer_getattr`` but handles its parameter
+  ``default`` like ``getattr``, i.e. it raises ``AttributeError``
+  if the attribute lookup fails and this parameter is not provided,
+  fixes `#287 <https://github.com/zopefoundation/RestrictedPython/issues/287>`_.
+  
+
 
 7.3 (2024-09-30)
 ----------------
@@ -16,7 +23,7 @@ Changes
   and give the same level of protection as direct attribute access in an
   environment based on ``RestrictedPython``'s ``safe_builtints``.
 - Prevent information leakage via ``AttributeError.obj``
-  and the ``string`` module.
+  and the ``string`` module. (CVE-2024-47532)
 
 
 7.2 (2024-08-02)
diff --git a/src/RestrictedPython/Guards.py b/src/RestrictedPython/Guards.py
index 5aa720b..ce092f0 100644
--- a/src/RestrictedPython/Guards.py
+++ b/src/RestrictedPython/Guards.py
@@ -240,6 +240,9 @@ def guarded_delattr(object, name):
 safe_builtins['delattr'] = guarded_delattr
 
 
+raise_ = object()
+
+
 def safer_getattr(object, name, default=None, getattr=getattr):
     """Getattr implementation which prevents using format on string objects.
 
@@ -263,12 +266,18 @@ def safer_getattr(object, name, default=None, getattr=getattr):
             '"{name}" is an invalid attribute name because it '
             'starts with "_"'.format(name=name)
         )
-    return getattr(object, name, default)
+    args = (object, name) + (() if default is raise_ else (default,))
+    return getattr(*args)
 
 
 safe_builtins['_getattr_'] = safer_getattr
 
 
+def safer_getattr_raise(object, name, default=raise_):
+    """like ``safer_getattr`` but raising ``AttributeError`` if failing."""
+    return safer_getattr(object, name, default)
+
+
 def guarded_iter_unpack_sequence(it, spec, _getiter_):
     """Protect sequence unpacking of targets in a 'for loop'.
 


Test patch:
diff --git a/tests/test_Guards.py b/tests/test_Guards.py
index f3db19b..b9c5b2f 100644
--- a/tests/test_Guards.py
+++ b/tests/test_Guards.py
@@ -329,6 +329,18 @@ def test_Guards__safer_getattr__5():
     ) == str(err.value)
 
 
+def test_Guards__safer_getattr_raise():
+    from types import SimpleNamespace
+
+    from RestrictedPython.Guards import safer_getattr_raise
+
+    o = SimpleNamespace(a="a")
+    assert safer_getattr_raise(o, "a") == "a"
+    assert safer_getattr_raise(o, "b", None) is None
+    with pytest.raises(AttributeError):
+        safer_getattr_raise(o, "b")
+
+
 def test_call_py3_builtins():
     """It should not be allowed to access global builtins in Python3."""
     result = compile_restricted_exec('builtins["getattr"]')