Skip to content

Commit 6c53af1

Browse files
a12kAaron Wieczorekpicnixz
authored
gh-143308: fix UAF when PickleBuffer is concurrently mutated in a callback (#143312)
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 ef3b882 commit 6c53af1

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
@@ -4261,6 +4261,35 @@ def check_array(arr):
42614261
# 2-D, non-contiguous
42624262
check_array(arr[::2])
42634263

4264+
def test_concurrent_mutation_in_buffer_with_bytearray(self):
4265+
def factory():
4266+
s = b"a" * 16
4267+
return bytearray(s), s
4268+
self.do_test_concurrent_mutation_in_buffer_callback(factory)
4269+
4270+
def test_concurrent_mutation_in_buffer_with_memoryview(self):
4271+
def factory():
4272+
obj = memoryview(b"a" * 32)[10:26]
4273+
sub = b"a" * len(obj)
4274+
return obj, sub
4275+
self.do_test_concurrent_mutation_in_buffer_callback(factory)
4276+
4277+
def do_test_concurrent_mutation_in_buffer_callback(self, factory):
4278+
# See: https://github.com/python/cpython/issues/143308.
4279+
class R:
4280+
def __bool__(self):
4281+
buf.release()
4282+
return True
4283+
4284+
for proto in range(5, pickle.HIGHEST_PROTOCOL + 1):
4285+
obj, sub = factory()
4286+
buf = pickle.PickleBuffer(obj)
4287+
buffer_callback = lambda _: R()
4288+
4289+
with self.subTest(proto=proto, obj=obj, sub=sub):
4290+
res = self.dumps(buf, proto, buffer_callback=buffer_callback)
4291+
self.assertIn(sub, res)
4292+
42644293
def test_evil_class_mutating_dict(self):
42654294
# https://github.com/python/cpython/issues/92930
42664295
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
@@ -2636,53 +2636,61 @@ save_picklebuffer(PickleState *st, PicklerObject *self, PyObject *obj)
26362636
"PickleBuffer can only be pickled with protocol >= 5");
26372637
return -1;
26382638
}
2639-
const Py_buffer* view = PyPickleBuffer_GetBuffer(obj);
2640-
if (view == NULL) {
2639+
Py_buffer view;
2640+
if (PyObject_GetBuffer(obj, &view, PyBUF_FULL_RO) != 0) {
26412641
return -1;
26422642
}
2643-
if (view->suboffsets != NULL || !PyBuffer_IsContiguous(view, 'A')) {
2643+
if (view.suboffsets != NULL || !PyBuffer_IsContiguous(&view, 'A')) {
26442644
PyErr_SetString(st->PicklingError,
26452645
"PickleBuffer can not be pickled when "
26462646
"pointing to a non-contiguous buffer");
2647-
return -1;
2647+
goto error;
26482648
}
2649+
2650+
int rc = 0;
26492651
int in_band = 1;
26502652
if (self->buffer_callback != NULL) {
26512653
PyObject *ret = PyObject_CallOneArg(self->buffer_callback, obj);
26522654
if (ret == NULL) {
2653-
return -1;
2655+
goto error;
26542656
}
26552657
in_band = PyObject_IsTrue(ret);
26562658
Py_DECREF(ret);
26572659
if (in_band == -1) {
2658-
return -1;
2660+
goto error;
26592661
}
26602662
}
26612663
if (in_band) {
26622664
/* Write data in-band */
2663-
if (view->readonly) {
2664-
return _save_bytes_data(st, self, obj, (const char *)view->buf,
2665-
view->len);
2665+
if (view.readonly) {
2666+
rc = _save_bytes_data(st, self, obj, (const char *)view.buf,
2667+
view.len);
26662668
}
26672669
else {
2668-
return _save_bytearray_data(st, self, obj, (const char *)view->buf,
2669-
view->len);
2670+
rc = _save_bytearray_data(st, self, obj, (const char *)view.buf,
2671+
view.len);
26702672
}
26712673
}
26722674
else {
26732675
/* Write data out-of-band */
26742676
const char next_buffer_op = NEXT_BUFFER;
26752677
if (_Pickler_Write(self, &next_buffer_op, 1) < 0) {
2676-
return -1;
2678+
goto error;
26772679
}
2678-
if (view->readonly) {
2680+
if (view.readonly) {
26792681
const char readonly_buffer_op = READONLY_BUFFER;
26802682
if (_Pickler_Write(self, &readonly_buffer_op, 1) < 0) {
2681-
return -1;
2683+
goto error;
26822684
}
26832685
}
26842686
}
2685-
return 0;
2687+
2688+
PyBuffer_Release(&view);
2689+
return rc;
2690+
2691+
error:
2692+
PyBuffer_Release(&view);
2693+
return -1;
26862694
}
26872695

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

0 commit comments

Comments
 (0)