gh-143308: fix UAF when PickleBuffer is concurrently mutated in a callback#143312
gh-143308: fix UAF when PickleBuffer is concurrently mutated in a callback#143312picnixz merged 13 commits intopython:mainfrom
Conversation
Misc/NEWS.d/next/Core_and_Builtins/2025-12-31-17-38-33.gh-issue-143308.lY8UCR.rst
Outdated
Show resolved
Hide resolved
|
Just a small thing to alter and we're good (good to keep a clickable ref) |
Co-authored-by: Bénédikt Tran <[email protected]>
Waiting until the NEWS entry has been moved.
Modules/_pickle.c
Outdated
|
|
||
| int rc = 0; | ||
| Py_buffer keepalive_view; // to ensure that 'view' is kept alive | ||
| if (PyObject_GetBuffer(view->obj, &keepalive_view, PyBUF_FULL_RO) != 0) { |
There was a problem hiding this comment.
Could not we simply apply PyObject_GetBuffer() to the PickleBuffer object itself? We can then get rid of PyPickleBuffer_GetBuffer().
There was a problem hiding this comment.
Potentially! I'd want to test it. @picnixz what do you think? Since this hasn't been merged yet now would seem to be the time.
There was a problem hiding this comment.
It should work. PyPickleBuffer_GetBuffer just returns a (borrowed) ref so it doesn't get any buffer on it. I think it should work so feel free to check!
There was a problem hiding this comment.
This worked great, thanks for the suggestion @serhiy-storchaka. @picnixz The change was a little more involved than expected, but by doing this I was able to remove a redundant view, update all the view-> calls, and the exit: label. All tests pass and confirmed that the patch still holds.
Modules/_pickle.c
Outdated
| view->len); | ||
| if (view.readonly) { | ||
| rc = _save_bytes_data(st, self, obj, (const char *)view.buf, | ||
| view.len); |
There was a problem hiding this comment.
Can you vertically align 'view.len' with 'st'? (likewise, some lines below)
| "PickleBuffer can not be pickled when " | ||
| "pointing to a non-contiguous buffer"); |
There was a problem hiding this comment.
Out of curiosity, but do we have a test for this?
There was a problem hiding this comment.
Neither of the tests added test for non-contiguous, but should be straightforward to add a test against a buffer with strides...
There was a problem hiding this comment.
I just wanted to be sure that this branch was covered by existing tests or by other tests. We can increase the coverage in a follow-up PR.
serhiy-storchaka
left a comment
There was a problem hiding this comment.
LGTM. 👍
In the following issue we can remove PyPickleBuffer_GetBuffer() and PyPickleBuffer_Release().
… a callback (pythonGH-143312) (cherry picked from commit 6c53af1) Co-authored-by: Aaron Wieczorek <[email protected]> Co-authored-by: Aaron Wieczorek <[email protected]> Co-authored-by: Bénédikt Tran <[email protected]>
… a callback (pythonGH-143312) (cherry picked from commit 6c53af1) Co-authored-by: Aaron Wieczorek <[email protected]> Co-authored-by: Aaron Wieczorek <[email protected]> Co-authored-by: Bénédikt Tran <[email protected]>
|
GH-143396 is a backport of this pull request to the 3.14 branch. |
|
GH-143397 is a backport of this pull request to the 3.13 branch. |
…n a callback (GH-143312) (#143396) gh-143308: fix UAF when PickleBuffer is concurrently mutated in a callback (GH-143312) (cherry picked from commit 6c53af1) --------------- Co-authored-by: Aaron Wieczorek <[email protected]> Co-authored-by: Aaron Wieczorek <[email protected]> Co-authored-by: Bénédikt Tran <[email protected]>
…n 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 <[email protected]> Co-authored-by: Aaron Wieczorek <[email protected]> Co-authored-by: Bénédikt Tran <[email protected]>
… a callback (python#143312) Co-authored-by: Aaron Wieczorek <[email protected]> Co-authored-by: Bénédikt Tran <[email protected]>
This PR addresses a use-after-free vulnerability in
save_picklebufferinModules/_pickle.c. The issue occurred when a buffer_callback explicitly released the PickleBuffer while the pickler was still serializing it.Validation
test_release_in_callback_keepaliveandtest_release_in_callback_complex_keepalivetoLib/test/test_pickle.py.Linked issue: gh-128038
save_picklebuffervia re-entrantbuffer_callbackand__bool__#143308