[Python-checkins] [3.11] gh-101765: Fix SystemError / segmentation fault in iter `__reduce__` when internal access of `builtins.__dict__` exhausts the iterator (GH-101769) (#102228)
JelleZijlstra
webhook-mailer at python.org
Fri Feb 24 22:50:29 EST 2023
https://github.com/python/cpython/commit/5d461225a5595bc682b5e9fd4cab41199304ff94
commit: 5d461225a5595bc682b5e9fd4cab41199304ff94
branch: 3.11
author: Ionite <dev at ionite.io>
committer: JelleZijlstra <jelle.zijlstra at gmail.com>
date: 2023-02-24T19:49:59-08:00
summary:
[3.11] gh-101765: Fix SystemError / segmentation fault in iter `__reduce__` when internal access of `builtins.__dict__` exhausts the iterator (GH-101769) (#102228)
(cherry picked from commit 54dfa14c5a94b893b67a4d9e9e403ff538ce9023)
files:
A Misc/NEWS.d/next/Core and Builtins/2023-02-10-07-21-47.gh-issue-101765.MO5LlC.rst
M Lib/test/test_iter.py
M Objects/bytearrayobject.c
M Objects/bytesobject.c
M Objects/genericaliasobject.c
M Objects/iterobject.c
M Objects/listobject.c
M Objects/tupleobject.c
M Objects/unicodeobject.c
diff --git a/Lib/test/test_iter.py b/Lib/test/test_iter.py
index acbdcb5f3020..6ab9b7a72303 100644
--- a/Lib/test/test_iter.py
+++ b/Lib/test/test_iter.py
@@ -7,6 +7,9 @@
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 @@ def __call__(self):
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 @@ def test_mutating_seq_class_exhausted_iter(self):
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 000000000000..cc99779a944e
--- /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 dfeed68416cf..6d46ebe2a448 100644
--- a/Objects/bytearrayobject.c
+++ b/Objects/bytearrayobject.c
@@ -2394,11 +2394,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 17a2661ef3be..8dd1a2d52467 100644
--- a/Objects/bytesobject.c
+++ b/Objects/bytesobject.c
@@ -3191,11 +3191,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 77acd1bc57a5..139bab7e6c03 100644
--- a/Objects/genericaliasobject.c
+++ b/Objects/genericaliasobject.c
@@ -885,8 +885,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 1732a037600c..ae09d2c0570e 100644
--- a/Objects/iterobject.c
+++ b/Objects/iterobject.c
@@ -103,11 +103,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.");
@@ -242,11 +247,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 19009133c827..0b20829a280a 100644
--- a/Objects/listobject.c
+++ b/Objects/listobject.c
@@ -3452,18 +3452,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));
listiterobject *it = (listiterobject *)_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 dfb8597b876e..1e82a5b674e0 100644
--- a/Objects/tupleobject.c
+++ b/Objects/tupleobject.c
@@ -1072,11 +1072,16 @@ PyDoc_STRVAR(length_hint_doc, "Private method returning an estimate of len(list(
static PyObject *
tupleiter_reduce(tupleiterobject *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 a19eaadcfc2e..02343d7c9326 100644
--- a/Objects/unicodeobject.c
+++ b/Objects/unicodeobject.c
@@ -15757,14 +15757,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 = (PyObject *)_PyUnicode_New(0);
if (u == NULL)
return NULL;
- return Py_BuildValue("N(N)", _PyEval_GetBuiltin(&_Py_ID(iter)), u);
+ return Py_BuildValue("N(N)", iter, u);
}
}
More information about the Python-checkins
mailing list