Skip to content

Commit

Permalink
gh-92930: _pickle.c: Acquire strong references before calling save() (G…
Browse files Browse the repository at this point in the history
…H-92931)

(cherry picked from commit 4c496f1)

Co-authored-by: Dennis Sweeney <36520290+sweeneyde@users.noreply.github.com>
  • Loading branch information
miss-islington and sweeneyde authored Jun 11, 2022
1 parent 0aa9ec9 commit 54fe3d5
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 11 deletions.
61 changes: 61 additions & 0 deletions Lib/test/pickletester.py
Original file line number Diff line number Diff line change
Expand Up @@ -3035,6 +3035,67 @@ def check_array(arr):
# 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:

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed a crash in ``_pickle.c`` from mutating collections during ``__reduce__`` or ``persistent_id``.
47 changes: 36 additions & 11 deletions Modules/_pickle.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 54fe3d5

Please sign in to comment.