diff --git a/Lib/test/test_pickle.py b/Lib/test/test_pickle.py index 48375cf459ea0b1..5729bebfc7887b7 100644 --- a/Lib/test/test_pickle.py +++ b/Lib/test/test_pickle.py @@ -382,6 +382,49 @@ class CUnpicklerTests(PyUnpicklerTests): truncated_data_error = (pickle.UnpicklingError, 'truncated') size_overflow_error = (OverflowError, 'exceeds') + def test_readinto_does_not_keep_buffer_alive(self): + # A readinto() that retains the memoryview it is handed must not be + # able to access the buffer after readinto() returns (gh-151046). + stashed = [] + + class StashingFile: + def __init__(self, data): + self._data = memoryview(data) + self._pos = 0 + + def read(self, n=-1): + if n is None or n < 0: + chunk = self._data[self._pos:] + else: + chunk = self._data[self._pos:self._pos + n] + self._pos += len(chunk) + return bytes(chunk) + + def readline(self): + return self.read(-1) + + def readinto(self, view): + stashed.append(view) + n = min(len(view), len(self._data) - self._pos) + view[:n] = self._data[self._pos:self._pos + n] + self._pos += n + return n + + # A large bytes object forces the file-read (readinto) path. + payload = b'spam' * 100_000 + data = pickle.dumps(payload, protocol=4) + obj = self.unpickler(StashingFile(data)).load() + self.assertEqual(obj, payload) + + self.assertTrue(stashed) + for view in stashed: + with self.assertRaises(ValueError): + view[0] + with self.assertRaises(ValueError): + view[0] = 0 + with self.assertRaises(ValueError): + bytes(view) + class CPicklingErrorTests(PyPicklingErrorTests): pickler = _pickle.Pickler diff --git a/Misc/NEWS.d/next/Library/2026-06-07-23-45-00.gh-issue-151046.S6dZlz.rst b/Misc/NEWS.d/next/Library/2026-06-07-23-45-00.gh-issue-151046.S6dZlz.rst new file mode 100644 index 000000000000000..2acb0d65c572050 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-06-07-23-45-00.gh-issue-151046.S6dZlz.rst @@ -0,0 +1,7 @@ +Fix a use-after-free in the C implementation of :mod:`pickle`. When +unpickling from a file-like object that provides ``readinto()``, the +:class:`~pickle.Unpickler` passed it a temporary :class:`memoryview` over an +internal buffer. A ``readinto()`` implementation that kept a reference to that +view could use it to read or write the buffer after it had been freed. The +view is now released as soon as ``readinto()`` returns, so a surviving +reference raises :exc:`ValueError` instead of accessing freed memory. diff --git a/Modules/_pickle.c b/Modules/_pickle.c index a5595be251a738d..74663a3e2a31866 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -1283,6 +1283,27 @@ _Unpickler_SkipConsumed(UnpicklerObject *self) static const Py_ssize_t READ_WHOLE_LINE = -1; +/* Release the temporary memoryview handed to readinto() and drop our reference, + so a view retained by readinto() can no longer reach the underlying buffer. + Returns -1 if release() failed. Any pending exception is preserved. */ +static int +_Unpickler_ReleaseBufObj(PyObject *buf_obj) +{ + PyObject *exc = PyErr_GetRaisedException(); + PyObject *res = PyObject_CallMethodNoArgs(buf_obj, &_Py_ID(release)); + int err = (res == NULL) ? -1 : 0; + Py_XDECREF(res); + if (exc != NULL) { + if (err < 0) { + PyErr_Clear(); + } + PyErr_SetRaisedException(exc); + err = 0; + } + Py_DECREF(buf_obj); + return err; +} + /* Don't call it directly: use _Unpickler_ReadInto() */ static Py_ssize_t _Unpickler_ReadIntoFromFile(PickleState *state, UnpicklerObject *self, char *buf, @@ -1318,17 +1339,24 @@ _Unpickler_ReadIntoFromFile(PickleState *state, UnpicklerObject *self, char *buf return n; } - /* Call readinto() into user buffer */ + /* buf is a temporary buffer; we wrap it in a memoryview only to pass it to + readinto(). Release the view once readinto() returns, so a view it kept + a reference to cannot be used to access buf after it is freed. */ PyObject *buf_obj = PyMemoryView_FromMemory(buf, n, PyBUF_WRITE); if (buf_obj == NULL) { return -1; } - PyObject *read_size_obj = _Pickle_FastCall(self->readinto, buf_obj); + /* PyObject_CallOneArg does not steal buf_obj, so we can release it below. */ + PyObject *read_size_obj = PyObject_CallOneArg(self->readinto, buf_obj); if (read_size_obj == NULL) { + _Unpickler_ReleaseBufObj(buf_obj); return -1; } Py_ssize_t read_size = PyLong_AsSsize_t(read_size_obj); Py_DECREF(read_size_obj); + if (_Unpickler_ReleaseBufObj(buf_obj) < 0) { + return -1; + } if (read_size < 0) { if (!PyErr_Occurred()) {