gh-135075: Make PyObject_SetAttr() fail with NULL value and exception (#136180)

vstinner · web-flow · commit da79ac9d2686 · 2025-07-03T14:51:44.000+02:00
Make PyObject_SetAttr() and PyObject_SetAttrString() fail if called
with NULL value and an exception set.
diff --git a/Doc/c-api/object.rst b/Doc/c-api/object.rst
@@ -197,6 +197,13 @@ Object Protocol
    in favour of using :c:func:`PyObject_DelAttr`, but there are currently no
    plans to remove it.
 
+   The function must not be called with ``NULL`` *v* and an an exception set.
+   This case can arise from forgetting ``NULL`` checks and would delete the
+   attribute.
+
+   .. versionchanged:: next
+      Must not be called with NULL value if an exception is set.
+
 
 .. c:function:: int PyObject_SetAttrString(PyObject *o, const char *attr_name, PyObject *v)
 
@@ -207,6 +214,10 @@ Object Protocol
    If *v* is ``NULL``, the attribute is deleted, but this feature is
    deprecated in favour of using :c:func:`PyObject_DelAttrString`.
 
+   The function must not be called with ``NULL`` *v* and an an exception set.
+   This case can arise from forgetting ``NULL`` checks and would delete the
+   attribute.
+
    The number of different attribute names passed to this function
    should be kept small, usually by using a statically allocated string
    as *attr_name*.
@@ -215,6 +226,10 @@ Object Protocol
    For more details, see :c:func:`PyUnicode_InternFromString`, which may be
    used internally to create a key object.
 
+   .. versionchanged:: next
+      Must not be called with NULL value if an exception is set.
+
+
 .. c:function:: int PyObject_GenericSetAttr(PyObject *o, PyObject *name, PyObject *value)
 
    Generic attribute setter and deleter function that is meant
diff --git a/Lib/test/test_capi/test_abstract.py b/Lib/test/test_capi/test_abstract.py
@@ -1077,6 +1077,31 @@ def test_iter_nextitem(self):
         with self.assertRaisesRegex(TypeError, regex):
             PyIter_NextItem(10)
 
+    def test_object_setattr_null_exc(self):
+        class Obj:
+            pass
+        obj = Obj()
+        obj.attr = 123
+
+        exc = ValueError("error")
+        with self.assertRaises(SystemError) as cm:
+            _testcapi.object_setattr_null_exc(obj, 'attr', exc)
+        self.assertIs(cm.exception.__context__, exc)
+        self.assertIsNone(cm.exception.__cause__)
+        self.assertHasAttr(obj, 'attr')
+
+        with self.assertRaises(SystemError) as cm:
+            _testcapi.object_setattrstring_null_exc(obj, 'attr', exc)
+        self.assertIs(cm.exception.__context__, exc)
+        self.assertIsNone(cm.exception.__cause__)
+        self.assertHasAttr(obj, 'attr')
+
+        with self.assertRaises(SystemError) as cm:
+            # undecodable name
+            _testcapi.object_setattrstring_null_exc(obj, b'\xff', exc)
+        self.assertIs(cm.exception.__context__, exc)
+        self.assertIsNone(cm.exception.__cause__)
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/Misc/NEWS.d/next/C_API/2025-07-01-16-22-39.gh-issue-135075.angu3J.rst b/Misc/NEWS.d/next/C_API/2025-07-01-16-22-39.gh-issue-135075.angu3J.rst
@@ -0,0 +1,2 @@
+Make :c:func:`PyObject_SetAttr` and :c:func:`PyObject_SetAttrString` fail if
+called with ``NULL`` value and an exception set. Patch by Victor Stinner.
diff --git a/Modules/_testcapi/abstract.c b/Modules/_testcapi/abstract.c
@@ -178,6 +178,42 @@ sequence_fast_get_item(PyObject *self, PyObject *args)
 }
 
 
+static PyObject *
+object_setattr_null_exc(PyObject *self, PyObject *args)
+{
+    PyObject *obj, *name, *exc;
+    if (!PyArg_ParseTuple(args, "OOO", &obj, &name, &exc)) {
+        return NULL;
+    }
+
+    PyErr_SetObject((PyObject*)Py_TYPE(exc), exc);
+    if (PyObject_SetAttr(obj, name, NULL) < 0) {
+        return NULL;
+    }
+    assert(PyErr_Occurred());
+    return NULL;
+}
+
+
+static PyObject *
+object_setattrstring_null_exc(PyObject *self, PyObject *args)
+{
+    PyObject *obj, *exc;
+    const char *name;
+    Py_ssize_t size;
+    if (!PyArg_ParseTuple(args, "Oz#O", &obj, &name, &size, &exc)) {
+        return NULL;
+    }
+
+    PyErr_SetObject((PyObject*)Py_TYPE(exc), exc);
+    if (PyObject_SetAttrString(obj, name, NULL) < 0) {
+        return NULL;
+    }
+    assert(PyErr_Occurred());
+    return NULL;
+}
+
+
 static PyMethodDef test_methods[] = {
     {"object_getoptionalattr", object_getoptionalattr, METH_VARARGS},
     {"object_getoptionalattrstring", object_getoptionalattrstring, METH_VARARGS},
@@ -191,6 +227,8 @@ static PyMethodDef test_methods[] = {
 
     {"sequence_fast_get_size", sequence_fast_get_size, METH_O},
     {"sequence_fast_get_item", sequence_fast_get_item, METH_VARARGS},
+    {"object_setattr_null_exc", object_setattr_null_exc, METH_VARARGS},
+    {"object_setattrstring_null_exc", object_setattrstring_null_exc, METH_VARARGS},
     {NULL},
 };
 
diff --git a/Objects/object.c b/Objects/object.c
@@ -1213,16 +1213,27 @@ PyObject_HasAttrString(PyObject *obj, const char *name)
 int
 PyObject_SetAttrString(PyObject *v, const char *name, PyObject *w)
 {
-    PyObject *s;
-    int res;
+    PyThreadState *tstate = _PyThreadState_GET();
+    if (w == NULL && _PyErr_Occurred(tstate)) {
+        PyObject *exc = _PyErr_GetRaisedException(tstate);
+        _PyErr_SetString(tstate, PyExc_SystemError,
+            "PyObject_SetAttrString() must not be called with NULL value "
+            "and an exception set");
+        _PyErr_ChainExceptions1Tstate(tstate, exc);
+        return -1;
+    }
 
-    if (Py_TYPE(v)->tp_setattr != NULL)
+    if (Py_TYPE(v)->tp_setattr != NULL) {
         return (*Py_TYPE(v)->tp_setattr)(v, (char*)name, w);
-    s = PyUnicode_InternFromString(name);
-    if (s == NULL)
+    }
+
+    PyObject *s = PyUnicode_InternFromString(name);
+    if (s == NULL) {
         return -1;
-    res = PyObject_SetAttr(v, s, w);
-    Py_XDECREF(s);
+    }
+
+    int res = PyObject_SetAttr(v, s, w);
+    Py_DECREF(s);
     return res;
 }
 
@@ -1440,6 +1451,16 @@ PyObject_HasAttr(PyObject *obj, PyObject *name)
 int
 PyObject_SetAttr(PyObject *v, PyObject *name, PyObject *value)
 {
+    PyThreadState *tstate = _PyThreadState_GET();
+    if (value == NULL && _PyErr_Occurred(tstate)) {
+        PyObject *exc = _PyErr_GetRaisedException(tstate);
+        _PyErr_SetString(tstate, PyExc_SystemError,
+            "PyObject_SetAttr() must not be called with NULL value "
+            "and an exception set");
+        _PyErr_ChainExceptions1Tstate(tstate, exc);
+        return -1;
+    }
+
     PyTypeObject *tp = Py_TYPE(v);
     int err;
 
@@ -1451,8 +1472,7 @@ PyObject_SetAttr(PyObject *v, PyObject *name, PyObject *value)
     }
     Py_INCREF(name);
 
-    PyInterpreterState *interp = _PyInterpreterState_GET();
-    _PyUnicode_InternMortal(interp, &name);
+    _PyUnicode_InternMortal(tstate->interp, &name);
     if (tp->tp_setattro != NULL) {
         err = (*tp->tp_setattro)(v, name, value);
         Py_DECREF(name);

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	+Make :c:func:`PyObject_SetAttr` and :c:func:`PyObject_SetAttrString` fail if
	`2`	+called with ``NULL`` value and an exception set. Patch by Victor Stinner.