diff --git a/Lib/test/test_marshal.py b/Lib/test/test_marshal.py index 78db4219e2997c..9ec37d27dfa39f 100644 --- a/Lib/test/test_marshal.py +++ b/Lib/test/test_marshal.py @@ -317,6 +317,133 @@ def test_recursion_limit(self): 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 00000000000000..d3242235c6059b --- /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 b60a36e128cd9f..dace22da0d4a94 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 @@ w_ref(PyObject *v, char *flag, WFILE *p) 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;