From 54dfa14c5a94b893b67a4d9e9e403ff538ce9023 Mon Sep 17 00:00:00 2001 From: Ionite Date: Fri, 24 Feb 2023 18:02:04 -0500 Subject: [PATCH] gh-101765: Fix SystemError / segmentation fault in iter `__reduce__` when internal access of `builtins.__dict__` exhausts the iterator (#101769) --- Lib/test/test_iter.py | 81 +++++++++++++++++++ ...-02-10-07-21-47.gh-issue-101765.MO5LlC.rst | 1 + Objects/bytearrayobject.c | 11 ++- Objects/bytesobject.c | 11 ++- Objects/genericaliasobject.c | 11 ++- Objects/iterobject.c | 22 +++-- Objects/listobject.c | 12 ++- Objects/tupleobject.c | 11 ++- Objects/unicodeobject.c | 11 ++- 9 files changed, 148 insertions(+), 23 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-02-10-07-21-47.gh-issue-101765.MO5LlC.rst diff --git a/Lib/test/test_iter.py b/Lib/test/test_iter.py index acbdcb5f302..6ab9b7a7230 100644 --- a/Lib/test/test_iter.py +++ b/Lib/test/test_iter.py @@ -7,6 +7,9 @@ from test.support.os_helper import TESTFN, unlink from test.support import check_free_after_iterating, ALWAYS_EQ, NEVER_EQ import pickle import collections.abc +import functools +import contextlib +import builtins # Test result of triple loop (too big to inline) TRIPLETS = [(0, 0, 0), (0, 0, 1), (0, 0, 2), @@ -91,6 +94,12 @@ class CallableIterClass: raise IndexError # Emergency stop return i +class EmptyIterClass: + def __len__(self): + return 0 + def __getitem__(self, i): + raise StopIteration + # Main test suite class TestCase(unittest.TestCase): @@ -238,6 +247,78 @@ class TestCase(unittest.TestCase): self.assertEqual(list(empit), [5, 6]) self.assertEqual(list(a), [0, 1, 2, 3, 4, 5, 6]) + def test_reduce_mutating_builtins_iter(self): + # This is a reproducer of issue #101765 + # where iter `__reduce__` calls could lead to a segfault or SystemError + # depending on the order of C argument evaluation, which is undefined + + # Backup builtins + builtins_dict = builtins.__dict__ + orig = {"iter": iter, "reversed": reversed} + + def run(builtin_name, item, sentinel=None): + it = iter(item) if sentinel is None else iter(item, sentinel) + + class CustomStr: + def __init__(self, name, iterator): + self.name = name + self.iterator = iterator + def __hash__(self): + return hash(self.name) + def __eq__(self, other): + # Here we exhaust our iterator, possibly changing + # its `it_seq` pointer to NULL + # The `__reduce__` call should correctly get + # the pointers after this call + list(self.iterator) + return other == self.name + + # del is required here + # to not prematurely call __eq__ from + # the hash collision with the old key + del builtins_dict[builtin_name] + builtins_dict[CustomStr(builtin_name, it)] = orig[builtin_name] + + return it.__reduce__() + + types = [ + (EmptyIterClass(),), + (bytes(8),), + (bytearray(8),), + ((1, 2, 3),), + (lambda: 0, 0), + (tuple[int],) # GenericAlias + ] + + try: + run_iter = functools.partial(run, "iter") + # The returned value of `__reduce__` should not only be valid + # but also *empty*, as `it` was exhausted during `__eq__` + # i.e "xyz" returns (iter, ("",)) + self.assertEqual(run_iter("xyz"), (orig["iter"], ("",))) + self.assertEqual(run_iter([1, 2, 3]), (orig["iter"], ([],))) + + # _PyEval_GetBuiltin is also called for `reversed` in a branch of + # listiter_reduce_general + self.assertEqual( + run("reversed", orig["reversed"](list(range(8)))), + (iter, ([],)) + ) + + for case in types: + self.assertEqual(run_iter(*case), (orig["iter"], ((),))) + finally: + # Restore original builtins + for key, func in orig.items(): + # need to suppress KeyErrors in case + # a failed test deletes the key without setting anything + with contextlib.suppress(KeyError): + # del is required here + # to not invoke our custom __eq__ from + # the hash collision with the old key + del builtins_dict[key] + builtins_dict[key] = func + # Test a new_style class with __iter__ but no next() method def test_new_style_iter_class(self): class IterClass(object): diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-02-10-07-21-47.gh-issue-101765.MO5LlC.rst b/Misc/NEWS.d/next/Core and Builtins/2023-02-10-07-21-47.gh-issue-101765.MO5LlC.rst new file mode 100644 index 00000000000..cc99779a944 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-02-10-07-21-47.gh-issue-101765.MO5LlC.rst @@ -0,0 +1 @@ +Fix SystemError / segmentation fault in iter ``__reduce__`` when internal access of ``builtins.__dict__`` keys mutates the iter object. diff --git a/Objects/bytearrayobject.c b/Objects/bytearrayobject.c index 072089e39aa..49d4dd52469 100644 --- a/Objects/bytearrayobject.c +++ b/Objects/bytearrayobject.c @@ -2391,11 +2391,16 @@ PyDoc_STRVAR(length_hint_doc, static PyObject * bytearrayiter_reduce(bytesiterobject *it, PyObject *Py_UNUSED(ignored)) { + PyObject *iter = _PyEval_GetBuiltin(&_Py_ID(iter)); + + /* _PyEval_GetBuiltin can invoke arbitrary code, + * call must be before access of iterator pointers. + * see issue #101765 */ + if (it->it_seq != NULL) { - return Py_BuildValue("N(O)n", _PyEval_GetBuiltin(&_Py_ID(iter)), - it->it_seq, it->it_index); + return Py_BuildValue("N(O)n", iter, it->it_seq, it->it_index); } else { - return Py_BuildValue("N(())", _PyEval_GetBuiltin(&_Py_ID(iter))); + return Py_BuildValue("N(())", iter); } } diff --git a/Objects/bytesobject.c b/Objects/bytesobject.c index ba2c2e978c6..657443f31fa 100644 --- a/Objects/bytesobject.c +++ b/Objects/bytesobject.c @@ -3169,11 +3169,16 @@ PyDoc_STRVAR(length_hint_doc, static PyObject * striter_reduce(striterobject *it, PyObject *Py_UNUSED(ignored)) { + PyObject *iter = _PyEval_GetBuiltin(&_Py_ID(iter)); + + /* _PyEval_GetBuiltin can invoke arbitrary code, + * call must be before access of iterator pointers. + * see issue #101765 */ + if (it->it_seq != NULL) { - return Py_BuildValue("N(O)n", _PyEval_GetBuiltin(&_Py_ID(iter)), - it->it_seq, it->it_index); + return Py_BuildValue("N(O)n", iter, it->it_seq, it->it_index); } else { - return Py_BuildValue("N(())", _PyEval_GetBuiltin(&_Py_ID(iter))); + return Py_BuildValue("N(())", iter); } } diff --git a/Objects/genericaliasobject.c b/Objects/genericaliasobject.c index 675fd496eef..888cb16edd1 100644 --- a/Objects/genericaliasobject.c +++ b/Objects/genericaliasobject.c @@ -877,8 +877,17 @@ ga_iter_clear(PyObject *self) { static PyObject * ga_iter_reduce(PyObject *self, PyObject *Py_UNUSED(ignored)) { + PyObject *iter = _PyEval_GetBuiltin(&_Py_ID(iter)); gaiterobject *gi = (gaiterobject *)self; - return Py_BuildValue("N(O)", _PyEval_GetBuiltin(&_Py_ID(iter)), gi->obj); + + /* _PyEval_GetBuiltin can invoke arbitrary code, + * call must be before access of iterator pointers. + * see issue #101765 */ + + if (gi->obj) + return Py_BuildValue("N(O)", iter, gi->obj); + else + return Py_BuildValue("N(())", iter); } static PyMethodDef ga_iter_methods[] = { diff --git a/Objects/iterobject.c b/Objects/iterobject.c index cfd6d0a7c95..149b701b68e 100644 --- a/Objects/iterobject.c +++ b/Objects/iterobject.c @@ -102,11 +102,16 @@ PyDoc_STRVAR(length_hint_doc, "Private method returning an estimate of len(list( static PyObject * iter_reduce(seqiterobject *it, PyObject *Py_UNUSED(ignored)) { + PyObject *iter = _PyEval_GetBuiltin(&_Py_ID(iter)); + + /* _PyEval_GetBuiltin can invoke arbitrary code, + * call must be before access of iterator pointers. + * see issue #101765 */ + if (it->it_seq != NULL) - return Py_BuildValue("N(O)n", _PyEval_GetBuiltin(&_Py_ID(iter)), - it->it_seq, it->it_index); + return Py_BuildValue("N(O)n", iter, it->it_seq, it->it_index); else - return Py_BuildValue("N(())", _PyEval_GetBuiltin(&_Py_ID(iter))); + return Py_BuildValue("N(())", iter); } PyDoc_STRVAR(reduce_doc, "Return state information for pickling."); @@ -239,11 +244,16 @@ calliter_iternext(calliterobject *it) static PyObject * calliter_reduce(calliterobject *it, PyObject *Py_UNUSED(ignored)) { + PyObject *iter = _PyEval_GetBuiltin(&_Py_ID(iter)); + + /* _PyEval_GetBuiltin can invoke arbitrary code, + * call must be before access of iterator pointers. + * see issue #101765 */ + if (it->it_callable != NULL && it->it_sentinel != NULL) - return Py_BuildValue("N(OO)", _PyEval_GetBuiltin(&_Py_ID(iter)), - it->it_callable, it->it_sentinel); + return Py_BuildValue("N(OO)", iter, it->it_callable, it->it_sentinel); else - return Py_BuildValue("N(())", _PyEval_GetBuiltin(&_Py_ID(iter))); + return Py_BuildValue("N(())", iter); } static PyMethodDef calliter_methods[] = { diff --git a/Objects/listobject.c b/Objects/listobject.c index ca6b7123113..494b40178f9 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -3444,18 +3444,22 @@ listiter_reduce_general(void *_it, int forward) { PyObject *list; + /* _PyEval_GetBuiltin can invoke arbitrary code, + * call must be before access of iterator pointers. + * see issue #101765 */ + /* the objects are not the same, index is of different types! */ if (forward) { + PyObject *iter = _PyEval_GetBuiltin(&_Py_ID(iter)); _PyListIterObject *it = (_PyListIterObject *)_it; if (it->it_seq) { - return Py_BuildValue("N(O)n", _PyEval_GetBuiltin(&_Py_ID(iter)), - it->it_seq, it->it_index); + return Py_BuildValue("N(O)n", iter, it->it_seq, it->it_index); } } else { + PyObject *reversed = _PyEval_GetBuiltin(&_Py_ID(reversed)); listreviterobject *it = (listreviterobject *)_it; if (it->it_seq) { - return Py_BuildValue("N(O)n", _PyEval_GetBuiltin(&_Py_ID(reversed)), - it->it_seq, it->it_index); + return Py_BuildValue("N(O)n", reversed, it->it_seq, it->it_index); } } /* empty iterator, create an empty list */ diff --git a/Objects/tupleobject.c b/Objects/tupleobject.c index 7d6d0e1bea2..6ee93ab5adc 100644 --- a/Objects/tupleobject.c +++ b/Objects/tupleobject.c @@ -1048,11 +1048,16 @@ PyDoc_STRVAR(length_hint_doc, "Private method returning an estimate of len(list( static PyObject * tupleiter_reduce(_PyTupleIterObject *it, PyObject *Py_UNUSED(ignored)) { + PyObject *iter = _PyEval_GetBuiltin(&_Py_ID(iter)); + + /* _PyEval_GetBuiltin can invoke arbitrary code, + * call must be before access of iterator pointers. + * see issue #101765 */ + if (it->it_seq) - return Py_BuildValue("N(O)n", _PyEval_GetBuiltin(&_Py_ID(iter)), - it->it_seq, it->it_index); + return Py_BuildValue("N(O)n", iter, it->it_seq, it->it_index); else - return Py_BuildValue("N(())", _PyEval_GetBuiltin(&_Py_ID(iter))); + return Py_BuildValue("N(())", iter); } static PyObject * diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c index f0c7aa7707f..6403e359c70 100644 --- a/Objects/unicodeobject.c +++ b/Objects/unicodeobject.c @@ -14784,14 +14784,19 @@ PyDoc_STRVAR(length_hint_doc, "Private method returning an estimate of len(list( static PyObject * unicodeiter_reduce(unicodeiterobject *it, PyObject *Py_UNUSED(ignored)) { + PyObject *iter = _PyEval_GetBuiltin(&_Py_ID(iter)); + + /* _PyEval_GetBuiltin can invoke arbitrary code, + * call must be before access of iterator pointers. + * see issue #101765 */ + if (it->it_seq != NULL) { - return Py_BuildValue("N(O)n", _PyEval_GetBuiltin(&_Py_ID(iter)), - it->it_seq, it->it_index); + return Py_BuildValue("N(O)n", iter, it->it_seq, it->it_index); } else { PyObject *u = unicode_new_empty(); if (u == NULL) return NULL; - return Py_BuildValue("N(N)", _PyEval_GetBuiltin(&_Py_ID(iter)), u); + return Py_BuildValue("N(N)", iter, u); } }