summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMiss Islington (bot) <31488909+miss-islington@users.noreply.github.com>2022-06-11 03:52:41 (GMT)
committerGitHub <noreply@github.com>2022-06-11 03:52:41 (GMT)
commit54fe3d57bf5078f2d019df20c2db8028eb1a9e0f (patch)
tree52dfca04646e6e98feec1e90b35b61336893b20d
parent0aa9ec9f5d1154319ec4332315fff373505b6373 (diff)
downloadcpython-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.py61
-rw-r--r--Misc/NEWS.d/next/Core and Builtins/2022-05-18-18-34-45.gh-issue-92930.kpYPOb.rst1
-rw-r--r--Modules/_pickle.c47
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;