diff options
author | Miss Islington (bot) <31488909+miss-islington@users.noreply.github.com> | 2022-06-11 03:52:41 (GMT) |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-06-11 03:52:41 (GMT) |
commit | 54fe3d57bf5078f2d019df20c2db8028eb1a9e0f (patch) | |
tree | 52dfca04646e6e98feec1e90b35b61336893b20d | |
parent | 0aa9ec9f5d1154319ec4332315fff373505b6373 (diff) | |
download | cpython-54fe3d57bf5078f2d019df20c2db8028eb1a9e0f.zip cpython-54fe3d57bf5078f2d019df20c2db8028eb1a9e0f.tar.gz cpython-54fe3d57bf5078f2d019df20c2db8028eb1a9e0f.tar.bz2 |
gh-92930: _pickle.c: Acquire strong references before calling save() (GH-92931)
(cherry picked from commit 4c496f1f115a7910d4606b4de233d14874c77bfa)
Co-authored-by: Dennis Sweeney <36520290+sweeneyde@users.noreply.github.com>
-rw-r--r-- | Lib/test/pickletester.py | 61 | ||||
-rw-r--r-- | Misc/NEWS.d/next/Core and Builtins/2022-05-18-18-34-45.gh-issue-92930.kpYPOb.rst | 1 | ||||
-rw-r--r-- | Modules/_pickle.c | 47 |
3 files changed, 98 insertions, 11 deletions
diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index d0ea7d0..21419e1 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -3035,6 +3035,67 @@ class AbstractPickleTests: # 2-D, non-contiguous check_array(arr[::2]) + def test_evil_class_mutating_dict(self): + # https://github.com/python/cpython/issues/92930 + from random import getrandbits + + global Bad + class Bad: + def __eq__(self, other): + return ENABLED + def __hash__(self): + return 42 + def __reduce__(self): + if getrandbits(6) == 0: + collection.clear() + return (Bad, ()) + + for proto in protocols: + for _ in range(20): + ENABLED = False + collection = {Bad(): Bad() for _ in range(20)} + for bad in collection: + bad.bad = bad + bad.collection = collection + ENABLED = True + try: + data = self.dumps(collection, proto) + self.loads(data) + except RuntimeError as e: + expected = "changed size during iteration" + self.assertIn(expected, str(e)) + + def test_evil_pickler_mutating_collection(self): + # https://github.com/python/cpython/issues/92930 + if not hasattr(self, "pickler"): + raise self.skipTest(f"{type(self)} has no associated pickler type") + + global Clearer + class Clearer: + pass + + def check(collection): + class EvilPickler(self.pickler): + def persistent_id(self, obj): + if isinstance(obj, Clearer): + collection.clear() + return None + pickler = EvilPickler(io.BytesIO(), proto) + try: + pickler.dump(collection) + except RuntimeError as e: + expected = "changed size during iteration" + self.assertIn(expected, str(e)) + + for proto in protocols: + check([Clearer()]) + check([Clearer(), Clearer()]) + check({Clearer()}) + check({Clearer(), Clearer()}) + check({Clearer(): 1}) + check({Clearer(): 1, Clearer(): 2}) + check({1: Clearer(), 2: Clearer()}) + class BigmemPickleTests: diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-05-18-18-34-45.gh-issue-92930.kpYPOb.rst b/Misc/NEWS.d/next/Core and Builtins/2022-05-18-18-34-45.gh-issue-92930.kpYPOb.rst new file mode 100644 index 0000000..cd5d7b3 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-05-18-18-34-45.gh-issue-92930.kpYPOb.rst @@ -0,0 +1 @@ +Fixed a crash in ``_pickle.c`` from mutating collections during ``__reduce__`` or ``persistent_id``. diff --git a/Modules/_pickle.c b/Modules/_pickle.c index 851fead..23d26f6 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -3006,7 +3006,10 @@ batch_list_exact(PicklerObject *self, PyObject *obj) if (PyList_GET_SIZE(obj) == 1) { item = PyList_GET_ITEM(obj, 0); - if (save(self, item, 0) < 0) + Py_INCREF(item); + int err = save(self, item, 0); + Py_DECREF(item); + if (err < 0) return -1; if (_Pickler_Write(self, &append_op, 1) < 0) return -1; @@ -3021,7 +3024,10 @@ batch_list_exact(PicklerObject *self, PyObject *obj) return -1; while (total < PyList_GET_SIZE(obj)) { item = PyList_GET_ITEM(obj, total); - if (save(self, item, 0) < 0) + Py_INCREF(item); + int err = save(self, item, 0); + Py_DECREF(item); + if (err < 0) return -1; total++; if (++this_batch == BATCHSIZE) @@ -3259,10 +3265,16 @@ batch_dict_exact(PicklerObject *self, PyObject *obj) /* Special-case len(d) == 1 to save space. */ if (dict_size == 1) { PyDict_Next(obj, &ppos, &key, &value); - if (save(self, key, 0) < 0) - return -1; - if (save(self, value, 0) < 0) - return -1; + Py_INCREF(key); + Py_INCREF(value); + if (save(self, key, 0) < 0) { + goto error; + } + if (save(self, value, 0) < 0) { + goto error; + } + Py_CLEAR(key); + Py_CLEAR(value); if (_Pickler_Write(self, &setitem_op, 1) < 0) return -1; return 0; @@ -3274,10 +3286,16 @@ batch_dict_exact(PicklerObject *self, PyObject *obj) if (_Pickler_Write(self, &mark_op, 1) < 0) return -1; while (PyDict_Next(obj, &ppos, &key, &value)) { - if (save(self, key, 0) < 0) - return -1; - if (save(self, value, 0) < 0) - return -1; + Py_INCREF(key); + Py_INCREF(value); + if (save(self, key, 0) < 0) { + goto error; + } + if (save(self, value, 0) < 0) { + goto error; + } + Py_CLEAR(key); + Py_CLEAR(value); if (++i == BATCHSIZE) break; } @@ -3292,6 +3310,10 @@ batch_dict_exact(PicklerObject *self, PyObject *obj) } while (i == BATCHSIZE); return 0; +error: + Py_XDECREF(key); + Py_XDECREF(value); + return -1; } static int @@ -3409,7 +3431,10 @@ save_set(PicklerObject *self, PyObject *obj) if (_Pickler_Write(self, &mark_op, 1) < 0) return -1; while (_PySet_NextEntry(obj, &ppos, &item, &hash)) { - if (save(self, item, 0) < 0) + Py_INCREF(item); + int err = save(self, item, 0); + Py_CLEAR(item); + if (err < 0) return -1; if (++i == BATCHSIZE) break; |