From: Serhiy Storchaka Date: Sat, 18 Apr 2026 08:24:33 +0000 (+0300) Subject: gh-148653: Fix some marshal errors related to recursive immutable objects (GH-148698) X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2e37d836411e99cff7bb341ba14be5ea95fac08c;p=thirdparty%2FPython%2Fcpython.git gh-148653: Fix some marshal errors related to recursive immutable objects (GH-148698) Forbid marshalling recursive code, slice and frozendict objects which cannot be correctly unmarshalled. Reject invalid marshal data produced by marshalling recursive frozendict objects which was previously incorrectly unmarshalled. Add multiple tests for recursive data structures. --- diff --git a/Lib/test/test_marshal.py b/Lib/test/test_marshal.py index 78db4219e299..9ec37d27dfa3 100644 --- a/Lib/test/test_marshal.py +++ b/Lib/test/test_marshal.py @@ -317,6 +317,133 @@ class BugsTestCase(unittest.TestCase): last.append([0]) self.assertRaises(ValueError, marshal.dumps, head) + def test_reference_loop_list(self): + a = [] + a.append(a) + for v in range(3): + self.assertRaises(ValueError, marshal.dumps, a, v) + for v in range(3, marshal.version + 1): + d = marshal.dumps(a, v) + b = marshal.loads(d) + self.assertIsInstance(b, list) + self.assertIs(b[0], b) + + def test_reference_loop_dict(self): + a = {} + a[None] = a + for v in range(3): + self.assertRaises(ValueError, marshal.dumps, a, v) + for v in range(3, marshal.version + 1): + d = marshal.dumps(a, v) + b = marshal.loads(d) + self.assertIsInstance(b, dict) + self.assertIs(b[None], b) + + def test_reference_loop_tuple(self): + a = ([],) + a[0].append(a) + for v in range(3): + self.assertRaises(ValueError, marshal.dumps, a, v) + for v in range(3, marshal.version + 1): + d = marshal.dumps(a, v) + b = marshal.loads(d) + self.assertIsInstance(b, tuple) + self.assertIsInstance(b[0], list) + self.assertIs(b[0][0], b) + + def test_reference_loop_code(self): + def f(): + return 1234.5 + code = f.__code__ + a = [] + code = code.replace(co_consts=code.co_consts + (a,)) + a.append(code) + for v in range(marshal.version + 1): + self.assertRaises(ValueError, marshal.dumps, code, v) + + def test_reference_loop_slice(self): + a = slice([], None) + a.start.append(a) + for v in range(marshal.version + 1): + self.assertRaises(ValueError, marshal.dumps, a, v) + + a = slice(None, []) + a.stop.append(a) + for v in range(marshal.version + 1): + self.assertRaises(ValueError, marshal.dumps, a, v) + + a = slice(None, None, []) + a.step.append(a) + for v in range(marshal.version + 1): + self.assertRaises(ValueError, marshal.dumps, a, v) + + def test_reference_loop_frozendict(self): + a = frozendict({None: []}) + a[None].append(a) + for v in range(marshal.version + 1): + self.assertRaises(ValueError, marshal.dumps, a, v) + + def test_loads_reference_loop_list(self): + data = b'\xdb\x01\x00\x00\x00r\x00\x00\x00\x00' # [] + a = marshal.loads(data) + self.assertIsInstance(a, list) + self.assertIs(a[0], a) + + def test_loads_reference_loop_dict(self): + data = b'\xfbNr\x00\x00\x00\x000' # {None: } + a = marshal.loads(data) + self.assertIsInstance(a, dict) + self.assertIs(a[None], a) + + def test_loads_abnormal_reference_loops(self): + # Indirect self-references of tuples. + data = b'\xa8\x01\x00\x00\x00[\x01\x00\x00\x00r\x00\x00\x00\x00' # ([],) + a = marshal.loads(data) + self.assertIsInstance(a, tuple) + self.assertIsInstance(a[0], list) + self.assertIs(a[0][0], a) + + data = b'\xa8\x01\x00\x00\x00{Nr\x00\x00\x00\x000' # ({None: },) + a = marshal.loads(data) + self.assertIsInstance(a, tuple) + self.assertIsInstance(a[0], dict) + self.assertIs(a[0][None], a) + + # Direct self-reference which cannot be created in Python. + data = b'\xa8\x01\x00\x00\x00r\x00\x00\x00\x00' # (,) + a = marshal.loads(data) + self.assertIsInstance(a, tuple) + self.assertIs(a[0], a) + + # Direct self-references which cannot be created in Python + # because of unhashability. + data = b'\xfbr\x00\x00\x00\x00N0' # {: None} + self.assertRaises(TypeError, marshal.loads, data) + data = b'\xbc\x01\x00\x00\x00r\x00\x00\x00\x00' # {} + self.assertRaises(TypeError, marshal.loads, data) + + for data in [ + # Indirect self-references of immutable objects. + b'\xba[\x01\x00\x00\x00r\x00\x00\x00\x00NN', # slice([], None) + b'\xbaN[\x01\x00\x00\x00r\x00\x00\x00\x00N', # slice(None, []) + b'\xbaNN[\x01\x00\x00\x00r\x00\x00\x00\x00', # slice(None, None, []) + b'\xba{Nr\x00\x00\x00\x000NN', # slice({None: }, None) + b'\xbaN{Nr\x00\x00\x00\x000N', # slice(None, {None: }) + b'\xbaNN{Nr\x00\x00\x00\x000', # slice(None, None, {None: }) + b'\xfdN[\x01\x00\x00\x00r\x00\x00\x00\x000', # frozendict({None: []}) + b'\xfdN{Nr\x00\x00\x00\x0000', # frozendict({None: {None: }) + + # Direct self-references which cannot be created in Python. + b'\xbe\x01\x00\x00\x00r\x00\x00\x00\x00', # frozenset({}) + b'\xfdNr\x00\x00\x00\x000', # frozendict({None: }) + b'\xfdr\x00\x00\x00\x00N0', # frozendict({: None}) + b'\xbar\x00\x00\x00\x00NN', # slice(, None) + b'\xbaNr\x00\x00\x00\x00N', # slice(None, ) + b'\xbaNNr\x00\x00\x00\x00', # slice(None, None, ) + ]: + with self.subTest(data=data): + self.assertRaises(ValueError, marshal.loads, data) + def test_exact_type_match(self): # Former bug: # >>> class Int(int): pass diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-04-17-20-37-02.gh-issue-148653.nbbHMh.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-04-17-20-37-02.gh-issue-148653.nbbHMh.rst new file mode 100644 index 000000000000..d3242235c605 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-04-17-20-37-02.gh-issue-148653.nbbHMh.rst @@ -0,0 +1,2 @@ +Forbid :mod:`marshalling ` recursive code objects, :class:`slice` +and :class:`frozendict` objects which cannot be correctly unmarshalled. diff --git a/Python/marshal.c b/Python/marshal.c index b60a36e128cd..dace22da0d4a 100644 --- a/Python/marshal.c +++ b/Python/marshal.c @@ -382,7 +382,6 @@ static int w_ref(PyObject *v, char *flag, WFILE *p) { _Py_hashtable_entry_t *entry; - int w; if (p->version < 3 || p->hashtable == NULL) return 0; /* not writing object references */ @@ -399,20 +398,28 @@ w_ref(PyObject *v, char *flag, WFILE *p) entry = _Py_hashtable_get_entry(p->hashtable, v); if (entry != NULL) { /* write the reference index to the stream */ - w = (int)(uintptr_t)entry->value; + uintptr_t w = (uintptr_t)entry->value; + if (w & 0x80000000LU) { + PyErr_Format(PyExc_ValueError, "cannot marshal recursion %T objects", v); + goto err; + } /* we don't store "long" indices in the dict */ - assert(0 <= w && w <= 0x7fffffff); + assert(w <= 0x7fffffff); w_byte(TYPE_REF, p); - w_long(w, p); + w_long((int)w, p); return 1; } else { - size_t s = p->hashtable->nentries; + size_t w = p->hashtable->nentries; /* we don't support long indices */ - if (s >= 0x7fffffff) { + if (w >= 0x7fffffff) { PyErr_SetString(PyExc_ValueError, "too many objects"); goto err; } - w = (int)s; + // Corresponding code should call w_complete() after + // writing the object. + if (PyCode_Check(v) || PySlice_Check(v) || PyFrozenDict_CheckExact(v)) { + w |= 0x80000000LU; + } if (_Py_hashtable_set(p->hashtable, Py_NewRef(v), (void *)(uintptr_t)w) < 0) { Py_DECREF(v); @@ -426,6 +433,27 @@ err: return 1; } +static void +w_complete(PyObject *v, WFILE *p) +{ + if (p->version < 3 || p->hashtable == NULL) { + return; + } + if (_PyObject_IsUniquelyReferenced(v)) { + return; + } + + _Py_hashtable_entry_t *entry = _Py_hashtable_get_entry(p->hashtable, v); + if (entry == NULL) { + return; + } + assert(entry != NULL); + uintptr_t w = (uintptr_t)entry->value; + assert(w & 0x80000000LU); + w &= ~0x80000000LU; + entry->value = (void *)(uintptr_t)w; +} + static void w_complex_object(PyObject *v, char flag, WFILE *p); @@ -599,6 +627,9 @@ w_complex_object(PyObject *v, char flag, WFILE *p) w_object(value, p); } w_object((PyObject *)NULL, p); + if (PyFrozenDict_CheckExact(v)) { + w_complete(v, p); + } } else if (PyAnySet_CheckExact(v)) { PyObject *value; @@ -684,6 +715,7 @@ w_complex_object(PyObject *v, char flag, WFILE *p) w_object(co->co_linetable, p); w_object(co->co_exceptiontable, p); Py_DECREF(co_code); + w_complete(v, p); } else if (PyObject_CheckBuffer(v)) { /* Write unknown bytes-like objects as a bytes object */ @@ -709,6 +741,7 @@ w_complex_object(PyObject *v, char flag, WFILE *p) w_object(slice->start, p); w_object(slice->stop, p); w_object(slice->step, p); + w_complete(v, p); } else { W_TYPE(TYPE_UNKNOWN, p); @@ -1433,9 +1466,19 @@ r_object(RFILE *p) case TYPE_DICT: case TYPE_FROZENDICT: v = PyDict_New(); - R_REF(v); - if (v == NULL) + if (v == NULL) { break; + } + if (type == TYPE_DICT) { + R_REF(v); + } + else { + idx = r_ref_reserve(flag, p); + if (idx < 0) { + Py_CLEAR(v); + break; + } + } for (;;) { PyObject *key, *val; key = r_object(p); @@ -1458,13 +1501,7 @@ r_object(RFILE *p) Py_CLEAR(v); } if (type == TYPE_FROZENDICT && v != NULL) { - PyObject *frozendict = PyFrozenDict_New(v); - if (frozendict != NULL) { - Py_SETREF(v, frozendict); - } - else { - Py_CLEAR(v); - } + Py_SETREF(v, PyFrozenDict_New(v)); } retval = v; break;