Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions Doc/c-api/object.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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*.
Expand All @@ -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
Expand Down
25 changes: 25 additions & 0 deletions Lib/test/test_capi/test_abstract.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
1 change: 1 addition & 0 deletions Lib/test/test_capi/test_object.py
Original file line number Diff line number Diff line change
Expand Up @@ -247,5 +247,6 @@ def func(x):

func(object())


if __name__ == "__main__":
unittest.main()
Original file line number Diff line number Diff line change
@@ -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.
38 changes: 38 additions & 0 deletions Modules/_testcapi/abstract.c
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -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},
};

Expand Down
38 changes: 29 additions & 9 deletions Objects/object.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;

Expand All @@ -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);
Expand Down
Loading