Skip to content

Commit 09e1e66

Browse files
authored
[3.14] gh-159185: Fix marshalling of a set modified while its elements are marshalled (GH-159195) (#159200)
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 865124c commit 09e1e66

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
@@ -562,6 +562,29 @@ def test_unmarshallable(self):
562562
with self.assertRaisesRegex(ValueError, "unmarshallable object"):
563563
marshal.dumps((arg, memoryview(b'')))
564564

565+
def test_set_changed_size(self):
566+
# A set is modified while its items are being marshalled.
567+
class Item:
568+
def __hash__(self):
569+
return 0
570+
def __buffer__(self, flags):
571+
change(sample)
572+
return memoryview(b'abc')
573+
def grow(sample):
574+
sample.update(range(1000, 1400))
575+
def shrink(sample):
576+
sample.difference_update(range(1, 60))
577+
for change in grow, shrink:
578+
for version in range(marshal.version + 1):
579+
with self.subTest(change=change.__name__, version=version):
580+
# The first item of the set modifies it.
581+
sample = {Item()}
582+
sample.update(range(1, 60))
583+
self.assertIsInstance(next(iter(sample)), Item)
584+
with self.assertRaisesRegex(RuntimeError,
585+
"set changed size"):
586+
marshal.dumps(sample, version)
587+
565588

566589
LARGE_SIZE = 2**31
567590
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
@@ -104,6 +104,7 @@ module marshal
104104
#define WFERR_NESTEDTOODEEP 2
105105
#define WFERR_NOMEMORY 3
106106
#define WFERR_CODE_NOT_ALLOWED 4
107+
#define WFERR_SET_CHANGED_SIZE 5
107108

108109
typedef struct {
109110
FILE *fp;
@@ -635,9 +636,19 @@ w_complex_object(PyObject *v, char flag, WFILE *p)
635636
p->error = WFERR_NOMEMORY;
636637
return;
637638
}
639+
// Marshalling an element can run arbitrary code (an audit hook, or
640+
// the __buffer__() method of the element), which can add elements
641+
// to the set or remove some: n is not trusted to still be the size
642+
// of the set.
638643
Py_ssize_t i = 0;
644+
bool changed = false;
639645
Py_BEGIN_CRITICAL_SECTION(v);
640646
while (_PySet_NextEntryRef(v, &pos, &value, &hash)) {
647+
if (i == n) {
648+
changed = true;
649+
Py_DECREF(value);
650+
break;
651+
}
641652
PyObject *dump = _PyMarshal_WriteObjectToString(value,
642653
p->version, p->allow_code);
643654
if (dump == NULL) {
@@ -659,7 +670,11 @@ w_complex_object(PyObject *v, char flag, WFILE *p)
659670
Py_DECREF(pairs);
660671
return;
661672
}
662-
assert(i == n);
673+
if (changed || i != n) {
674+
p->error = WFERR_SET_CHANGED_SIZE;
675+
Py_DECREF(pairs);
676+
return;
677+
}
663678
if (PyList_Sort(pairs)) {
664679
p->error = WFERR_NOMEMORY;
665680
Py_DECREF(pairs);
@@ -1909,6 +1924,10 @@ _PyMarshal_WriteObjectToString(PyObject *x, int version, int allow_code)
19091924
PyErr_SetString(PyExc_ValueError,
19101925
"marshalling code objects is disallowed");
19111926
break;
1927+
case WFERR_SET_CHANGED_SIZE:
1928+
PyErr_SetString(PyExc_RuntimeError,
1929+
"set changed size during iteration");
1930+
break;
19121931
default:
19131932
case WFERR_UNMARSHALLABLE:
19141933
PyErr_SetString(PyExc_ValueError,

0 commit comments

Comments
 (0)