Skip to content

Commit cd71799

Browse files
authored
gh-159185: Fix marshalling of a set modified while its elements are marshalled (GH-159195)
Marshalling a set element can run Python code (__buffer__, an audit hook) which changes the set. The number of elements was read before the loop, so a set which grew was written past the end of the list and one which shrank left NULL items in the list which was then sorted. Raise RuntimeError("set changed size during iteration") instead.
1 parent e878e1e commit cd71799

3 files changed

Lines changed: 43 additions & 1 deletion

File tree

‎Lib/test/test_marshal.py‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -605,6 +605,30 @@ def test_error_in_set_item(self):
605605
with self.assertRaisesRegex(TypeError, "'<' not supported"):
606606
marshal.dumps({complex(nan, 0), complex(nan, 0)})
607607

608+
def test_set_changed_size(self):
609+
# A set is modified while its items are being marshalled.
610+
class Item:
611+
def __hash__(self):
612+
return 0
613+
def __buffer__(self, flags):
614+
change(sample)
615+
return memoryview(b'abc')
616+
def grow(sample):
617+
sample.update(range(1000, 1400))
618+
def shrink(sample):
619+
sample.difference_update(range(1, 60))
620+
for change in grow, shrink:
621+
for version in range(marshal.version + 1):
622+
with self.subTest(change=change.__name__, version=version):
623+
# The first item of the set modifies it.
624+
sample = {Item()}
625+
sample.update(range(1, 60))
626+
self.assertIsInstance(next(iter(sample)), Item)
627+
with self.assertRaisesRegex(RuntimeError,
628+
"set changed size"):
629+
marshal.dumps(sample, version)
630+
631+
608632
def test_error_in_buffer(self):
609633
# The BufferError raised for a non-contiguous buffer is not replaced
610634
# with a generic error.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix a crash in :func:`marshal.dumps` and :func:`marshal.dump` when a set is
2+
modified while its elements are serialized, for example by the
3+
:meth:`~object.__buffer__` method of one of its elements.
4+
:exc:`RuntimeError` is now raised.

‎Python/marshal.c‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -681,9 +681,19 @@ w_complex_object(PyObject *v, char flag, WFILE *p)
681681
p->error = true;
682682
return;
683683
}
684+
// Marshalling an element can run arbitrary code (an audit hook, or
685+
// the __buffer__() method of the element), which can add elements
686+
// to the set or remove some: n is not trusted to still be the size
687+
// of the set.
684688
Py_ssize_t i = 0;
689+
bool changed = false;
685690
Py_BEGIN_CRITICAL_SECTION(v);
686691
while (_PySet_NextEntryRef(v, &pos, &value, &hash)) {
692+
if (i == n) {
693+
changed = true;
694+
Py_DECREF(value);
695+
break;
696+
}
687697
PyObject *dump = _PyMarshal_WriteObjectToString(value,
688698
p->version, p->allow_code);
689699
if (dump == NULL) {
@@ -699,11 +709,15 @@ w_complex_object(PyObject *v, char flag, WFILE *p)
699709
PyList_SET_ITEM(pairs, i++, pair);
700710
}
701711
Py_END_CRITICAL_SECTION();
712+
if (!p->error && (changed || i != n)) {
713+
PyErr_SetString(PyExc_RuntimeError,
714+
"set changed size during iteration");
715+
p->error = true;
716+
}
702717
if (p->error) {
703718
Py_DECREF(pairs);
704719
return;
705720
}
706-
assert(i == n);
707721
if (PyList_Sort(pairs)) {
708722
p->error = true;
709723
Py_DECREF(pairs);

0 commit comments

Comments
 (0)