8000 gh-92930: _pickle.c: Acquire strong references before calling save() … · python/cpython@54fe3d5 · GitHub
[go: up one dir, main page]
Skip to content

Navigation Menu

Sign in

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 54fe3d5

Browse files
miss-islingtonsweeneyde
miss-islington
and
sweeneyde
authored
gh-92930: _pickle.c: Acquire strong references before calling save() (GH-92931)
(cherry picked from commit 4c496f1) Co-authored-by: Dennis Sweeney <36520290+sweeneyde@users.noreply.github.com>
1 parent 0aa9ec9 commit 54fe3d5

File tree

3 files changed

+98
-11
lines changed

3 files changed

+98
-11
lines changed

‎Lib/test/pickletester.py

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3035,6 +3035,67 @@ def check_array(arr):
30353035
# 2-D, non-contiguous
30363036
check_array(arr[::2])
30373037

3038+
def test_evil_class_mutating_dict(self):
3039+
# https://github.com/python/cpython/issues/92930
3040+
from random import getrandbits
3041+
3042+
global Bad
3043+
class Bad:
3044+
def __eq__(self, other):
3045+
return ENABLED
3046+
def __hash__(self):
3047+
return 42
3048+
def __reduce__(self):
3049+
if getrandbits(6) == 0:
3050+
collection.clear()
3051+
return (Bad, ())
3052+
3053+
for proto in protocols:
3054+
for _ in range(20):
3055+
ENABLED = False
3056+
collection = {Bad(): Bad() for _ in range(20)}
3057+
for bad in collection:
3058+
bad.bad = bad
3059+
bad.collection = collection
3060+
ENABLED = True
3061+
try:
3062+
data = self.dumps(collection, proto)
3063+
self.loads(data)
3064+
except RuntimeError as e:
3065+
expected = "changed size during iteration"
3066+
self.assertIn(expected, str(e))
3067+
3068+
def test_evil_pickler_mutating_collection(self):
3069+
# https://github.com/python/cpython/issues/92930
3070+
if not hasattr(self, "pickler"):
3071+
raise self.skipTest(f"{type(self)} has no associated pickler type")
3072+
3073+
global Clearer
3074+
class Clearer:
3075+
pass
3076+
3077+
def check(collection):
3078+
class EvilPickler(self.pickler):
3079+
def persistent_id(self, obj):
3080+
if isinstance(obj, Clearer):
3081+
collection.clear()
3082+
return None
3083+
pickler = EvilPickler(io.BytesIO(), proto)
3084+
try:
3085+
pickler.dump(collection)
3086+
except RuntimeError as e:
3087+
expected = "changed size during iteration"
3088+
self.assertIn(expected, str(e))
3089+
3090+
for proto in protocols:
3091+
check([Clearer()])
3092+
check([Clearer(), Clearer()])
3093+
check({Clearer()})
3094+
check({Clearer(), Clearer()})
3095+
check({Clearer(): 1})
3096+
check({Clearer(): 1, Clearer(): 2})
3097+
check({1: Clearer(), 2: Clearer()})
3098+
30383099

30393100
class BigmemPickleTests:
30403101

‎Misc/NEWS.d/next/Core and Builtins/2022-05-18-18-34-45.gh-issue-92930.kpYPOb.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fixed a crash in ``_pickle.c`` from mutating collections during ``__reduce__`` or ``persistent_id``.

‎Modules/_pickle.c

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3006,7 +3006,10 @@ batch_list_exact(PicklerObject *self, PyObject *obj)
30063006

30073007
if (PyList_GET_SIZE(obj) == 1) {
30083008
item = PyList_GET_ITEM(obj, 0);
3009-
if (save(self, item, 0) < 0)
3009+
Py_INCREF(item);
3010+
int err = save(self, item, 0);
3011+
Py_DECREF(item);
3012+
if (err < 0)
30103013
return -1;
30113014
if (_Pickler_Write(self, &append_op, 1) < 0)
30123015
return -1;
@@ -3021,7 +3024,10 @@ batch_list_exact(PicklerObject *self, PyObject *obj)
30213024
return -1;
30223025
while (total < PyList_GET_SIZE(obj)) {
30233026
item = PyList_GET_ITEM(obj, total);
3024-
if (save(self, item, 0) < 0)
3027+
Py_INCREF(item);
3028+
int err = save(self, item, 0);
3029+
Py_DECREF(item);
3030+
if (err < 0)
30253031
return -1;
30263032
total++;
30273033
if (++this_batch == BATCHSIZE)
@@ -3259,10 +3265,16 @@ batch_dict_exact(PicklerObject *self, PyObject *obj)
32593265
/* Special-case len(d) == 1 to save space. */
32603266
if (dict_size == 1) {
32613267
PyDict_Next(obj, &ppos, &key, &value);
3262-
if (save(self, key, 0) < 0)
3263-
return -1;
3264-
if (save(self, value, 0) < 0)
3265-
return -1;
3268+
Py_INCREF(key);
3269+
Py_INCREF(value);
3270+
if (save(self, key, 0) < 0) {
3271+
goto error;
3272+
}
3273+
if (save(self, value, 0) < 0) {
3274+
goto error;
3275+
}
3276+
Py_CLEAR(key);
3277+
Py_CLEAR(value);
32663278
if (_Pickler_Write(self, &setitem_op, 1) < 0)
32673279
return -1;
32683280
return 0;
@@ -3274,10 +3286,16 @@ batch_dict_exact(PicklerObject *self, PyObject *obj)
32743286
if (_Pickler_Write(self, &mark_op, 1) < 0)
32753287
return -1;
32763288
while (PyDict_Next(obj, &ppos, &key, &value)) {
3277-
if (save(self, key, 0) < 0)
3278-
return -1;
3279-
if (save(self, value, 0) < 0)
3280-
return -1;
3289+
Py_INCREF(key);
3290+
Py_INCREF(value);
3291+
if (save(self, key, 0) < 0) {
3292+
goto error;
3293+
}
3294+
if (save(self, value, 0) < 0) {
3295+
goto error;
3296+
}
3297+
Py_CLEAR(key);
3298+
Py_CLEAR(value);
32813299
if (++i == BATCHSIZE)
32823300
break;
32833301
}
@@ -3292,6 +3310,10 @@ batch_dict_exact(PicklerObject *self, PyObject *obj)
32923310

32933311
} while (i == BATCHSIZE);
32943312
return 0;
3313+
error:
3314+
Py_XDECREF(key);
3315+
Py_XDECREF(value);
3316+
return -1;
32953317
}
32963318

32973319
static int
@@ -3409,7 +3431,10 @@ save_set(PicklerObject *self, PyObject *obj)
34093431
if (_Pickler_Write(self, &mark_op, 1) < 0)
34103432
return -1;
34113433
while (_PySet_NextEntry(obj, &ppos, &item, &hash)) {
3412-
if (save(self, item, 0) < 0)
3434+
Py_INCREF(item);
3435+
int err = save(self, item, 0);
3436+
Py_CLEAR(item);
3437+
if (err < 0)
34133438
return -1;
34143439
if (++i == BATCHSIZE)
34153440
break;

0 commit comments

Comments
 (0)
0