Skip to content

Commit 2b8ac2c

Browse files
miss-islingtona12kAaron Wieczorekpicnixz
authored
[3.13] gh-143308: fix UAF when PickleBuffer is concurrently mutated in a callback (GH-143312) (#143397)
gh-143308: fix UAF when PickleBuffer is concurrently mutated in a callback (GH-143312) (cherry picked from commit 6c53af1) --------------- Co-authored-by: Aaron Wieczorek <aaronw@fastmail.com> Co-authored-by: Aaron Wieczorek <woz@Aarons-MacBook-Pro.local> Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
1 parent dced1a7 commit 2b8ac2c

File tree

3 files changed

+55
-15
lines changed

3 files changed

+55
-15
lines changed

Lib/test/pickletester.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3761,6 +3761,35 @@ def check_array(arr):
37613761
# 2-D, non-contiguous
37623762
check_array(arr[::2])
37633763

3764+
def test_concurrent_mutation_in_buffer_with_bytearray(self):
3765+
def factory():
3766+
s = b"a" * 16
3767+
return bytearray(s), s
3768+
self.do_test_concurrent_mutation_in_buffer_callback(factory)
3769+
3770+
def test_concurrent_mutation_in_buffer_with_memoryview(self):
3771+
def factory():
3772+
obj = memoryview(b"a" * 32)[10:26]
3773+
sub = b"a" * len(obj)
3774+
return obj, sub
3775+
self.do_test_concurrent_mutation_in_buffer_callback(factory)
3776+
3777+
def do_test_concurrent_mutation_in_buffer_callback(self, factory):
3778+
# See: https://github.com/python/cpython/issues/143308.
3779+
class R:
3780+
def __bool__(self):
3781+
buf.release()
3782+
return True
3783+
3784+
for proto in range(5, pickle.HIGHEST_PROTOCOL + 1):
3785+
obj, sub = factory()
3786+
buf = pickle.PickleBuffer(obj)
3787+
buffer_callback = lambda _: R()
3788+
3789+
with self.subTest(proto=proto, obj=obj, sub=sub):
3790+
res = self.dumps(buf, proto, buffer_callback=buffer_callback)
3791+
self.assertIn(sub, res)
3792+
37643793
def test_evil_class_mutating_dict(self):
37653794
# https://github.com/python/cpython/issues/92930
37663795
from random import getrandbits
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
:mod:`pickle`: fix use-after-free crashes when a :class:`~pickle.PickleBuffer`
2+
is concurrently mutated by a custom buffer callback during pickling.
3+
Patch by Bénédikt Tran and Aaron Wieczorek.

Modules/_pickle.c

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2523,53 +2523,61 @@ save_picklebuffer(PickleState *st, PicklerObject *self, PyObject *obj)
25232523
"PickleBuffer can only be pickled with protocol >= 5");
25242524
return -1;
25252525
}
2526-
const Py_buffer* view = PyPickleBuffer_GetBuffer(obj);
2527-
if (view == NULL) {
2526+
Py_buffer view;
2527+
if (PyObject_GetBuffer(obj, &view, PyBUF_FULL_RO) != 0) {
25282528
return -1;
25292529
}
2530-
if (view->suboffsets != NULL || !PyBuffer_IsContiguous(view, 'A')) {
2530+
if (view.suboffsets != NULL || !PyBuffer_IsContiguous(&view, 'A')) {
25312531
PyErr_SetString(st->PicklingError,
25322532
"PickleBuffer can not be pickled when "
25332533
"pointing to a non-contiguous buffer");
2534-
return -1;
2534+
goto error;
25352535
}
2536+
2537+
int rc = 0;
25362538
int in_band = 1;
25372539
if (self->buffer_callback != NULL) {
25382540
PyObject *ret = PyObject_CallOneArg(self->buffer_callback, obj);
25392541
if (ret == NULL) {
2540-
return -1;
2542+
goto error;
25412543
}
25422544
in_band = PyObject_IsTrue(ret);
25432545
Py_DECREF(ret);
25442546
if (in_band == -1) {
2545-
return -1;
2547+
goto error;
25462548
}
25472549
}
25482550
if (in_band) {
25492551
/* Write data in-band */
2550-
if (view->readonly) {
2551-
return _save_bytes_data(st, self, obj, (const char *)view->buf,
2552-
view->len);
2552+
if (view.readonly) {
2553+
rc = _save_bytes_data(st, self, obj, (const char *)view.buf,
2554+
view.len);
25532555
}
25542556
else {
2555-
return _save_bytearray_data(st, self, obj, (const char *)view->buf,
2556-
view->len);
2557+
rc = _save_bytearray_data(st, self, obj, (const char *)view.buf,
2558+
view.len);
25572559
}
25582560
}
25592561
else {
25602562
/* Write data out-of-band */
25612563
const char next_buffer_op = NEXT_BUFFER;
25622564
if (_Pickler_Write(self, &next_buffer_op, 1) < 0) {
2563-
return -1;
2565+
goto error;
25642566
}
2565-
if (view->readonly) {
2567+
if (view.readonly) {
25662568
const char readonly_buffer_op = READONLY_BUFFER;
25672569
if (_Pickler_Write(self, &readonly_buffer_op, 1) < 0) {
2568-
return -1;
2570+
goto error;
25692571
}
25702572
}
25712573
}
2572-
return 0;
2574+
2575+
PyBuffer_Release(&view);
2576+
return rc;
2577+
2578+
error:
2579+
PyBuffer_Release(&view);
2580+
return -1;
25732581
}
25742582

25752583
/* A copy of PyUnicode_AsRawUnicodeEscapeString() that also translates

0 commit comments

Comments
 (0)