gh-151046: Fix use-after-free in _Unpickler_ReadIntoFromFile#151048
Open
tonghuaroot wants to merge 2 commits into
Open
gh-151046: Fix use-after-free in _Unpickler_ReadIntoFromFile#151048tonghuaroot wants to merge 2 commits into
tonghuaroot wants to merge 2 commits into
Conversation
When unpickling from a file-like object that provides readinto(), the C Unpickler handed it a temporary memoryview over an internal buffer and never released it. A readinto() implementation that kept a reference to the view could read or write the buffer after it had been freed, a use-after-free reachable from pure Python. Keep an owned reference across the call and release the memoryview as soon as readinto() returns, so a surviving reference raises ValueError instead of dereferencing freed memory. Add a regression test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When the C
pickle.Unpicklerreads from a file-like object withreadinto(),it wraps an internal, short-lived buffer in a
memoryviewand passes it toreadinto(). The view was never released after the call, so areadinto()that retains the view could read or write the buffer after it was freed -- a
C-level use-after-free reachable from pure Python.
This keeps our own reference across the call (using
PyObject_CallOneArginstead of the reference-stealing
_Pickle_FastCall) and releases thememoryview as soon as
readinto()returns. A surviving reference now raisesValueError: operation forbidden on released memoryview objectinstead oftouching freed memory.
A well-behaved
readinto()releases any view it acquired before it returns, soby then the buffer's export count is already zero and the added
release()is ano-op -- ordinary file objects round-trip unchanged. The release only has an
effect when a
readinto()improperly keeps the view alive. Thereadinto()fast path dates back to bpo-39681, so the issue is present on all maintained
branches.
Adds a regression test (
CUnpicklerTests.test_readinto_does_not_keep_buffer_alive)that fails on the unpatched build and a
Misc/NEWS.dentry.AI Usage Statement
The discovery, ASAN reproduction, root-cause analysis, and review of the fix
and tests were done by the contributor. An AI assistant helped draft the
implementation and the regression test under the contributor's direction. All
changes were verified by hand (ASAN reproduction before/after, full
test_pickle/test_memoryview/test_picklebuffer/test_ioruns). PerCPython's policy, disclosure of AI assistance is appreciated and is made here.