Skip to content

Commit 4c2433f

Browse files
authored
[3.15] gh-159185: Fix marshalling of a set modified while its elements are marshalled (GH-159195) (#159199)
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. (cherry picked from commit cd71799)
1 parent b21f041 commit 4c2433f

3 files changed

Lines changed: 47 additions & 1 deletion

File tree

‎Lib/test/test_marshal.py‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,6 +582,29 @@ def test_unmarshallable(self):
582582
with self.assertRaisesRegex(ValueError, "unmarshallable object"):
583583
marshal.dumps((arg, memoryview(b'')))
584584

585+
def test_set_changed_size(self):
586+
# A set is modified while its items are being marshalled.
587+
class Item:
588+
def __hash__(self):
589+
return 0
590+
def __buffer__(self, flags):
591+
change(sample)
592+
return memoryview(b'abc')
593+
def grow(sample):
594+
sample.update(range(1000, 1400))
595+
def shrink(sample):
596+
sample.difference_update(range(1, 60))
597+
for change in grow, shrink:
598+
for version in range(marshal.version + 1):
599+
with self.subTest(change=change.__name__, version=version):
600+
# The first item of the set modifies it.
601+
sample = {Item()}
602+
sample.update(range(1, 60))
603+
self.assertIsInstance(next(iter(sample)), Item)
604+
with self.assertRaisesRegex(RuntimeError,
605+
"set changed size"):
606+
marshal.dumps(sample, version)
607+
585608

586609
LARGE_SIZE = 2**31
587610
pointer_size = 8 if sys.maxsize > 0xFFFFFFFF else 4
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: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ module marshal
106106
#define WFERR_NESTEDTOODEEP 2
107107
#define WFERR_NOMEMORY 3
108108
#define WFERR_CODE_NOT_ALLOWED 4
109+
#define WFERR_SET_CHANGED_SIZE 5
109110

110111
typedef struct {
111112
FILE *fp;
@@ -651,9 +652,19 @@ w_complex_object(PyObject *v, char flag, WFILE *p)
651652
p->error = WFERR_NOMEMORY;
652653
return;
653654
}
655+
// Marshalling an element can run arbitrary code (an audit hook, or
656+
// the __buffer__() method of the element), which can add elements
657+
// to the set or remove some: n is not trusted to still be the size
658+
// of the set.
654659
Py_ssize_t i = 0;
660+
bool changed = false;
655661
Py_BEGIN_CRITICAL_SECTION(v);
656662
while (_PySet_NextEntryRef(v, &pos, &value, &hash)) {
663+
if (i == n) {
664+
changed = true;
665+
Py_DECREF(value);
666+
break;
667+
}
657668
PyObject *dump = _PyMarshal_WriteObjectToString(value,
658669
p->version, p->allow_code);
659670
if (dump == NULL) {
@@ -673,7 +684,11 @@ w_complex_object(PyObject *v, char flag, WFILE *p)
673684
Py_DECREF(pairs);
674685
return;
675686
}
676-
assert(i == n);
687+
if (changed || i != n) {
688+
p->error = WFERR_SET_CHANGED_SIZE;
689+
Py_DECREF(pairs);
690+
return;
691+
}
677692
if (PyList_Sort(pairs)) {
678693
p->error = WFERR_NOMEMORY;
679694
Py_DECREF(pairs);
@@ -1941,6 +1956,10 @@ _PyMarshal_WriteObjectToString(PyObject *x, int version, int allow_code)
19411956
PyErr_SetString(PyExc_ValueError,
19421957
"marshalling code objects is disallowed");
19431958
break;
1959+
case WFERR_SET_CHANGED_SIZE:
1960+
PyErr_SetString(PyExc_RuntimeError,
1961+
"set changed size during iteration");
1962+
break;
19441963
default:
19451964
case WFERR_UNMARSHALLABLE:
19461965
PyErr_SetString(PyExc_ValueError,

0 commit comments

Comments
 (0)