| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
For non-contiguous input, get_data_from_buffer() makes a temporary contiguous copy and buf points into it. unpackb() released the buffer view (freeing that copy) in the finally block, and only afterwards read buf+off to build the ExtraData payload, so ExtraData.extra was filled from freed memory. Copy the extra data out before releasing the view. Fixes msgpack#720
|
^ made with Claude Opus 5, please review carefully. |
Sorry, something went wrong.
|
Thanks for the fix! |
Sorry, something went wrong.
|
One leftover: my cdef object extra = None at /Users/tw/w/msgpack-python/msgpack/_unpacker.pyx:171 survived the refactor and is now unused. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Fixes #720.
Problem
For non-contiguous input, get_data_from_buffer() releases the original view and makes a temporary contiguous copy, so buf points into memory owned by view:
https://github.com/msgpack/msgpack-python/blob/main/msgpack/_unpacker.pyx#L129-L134
unpackb() then releases view in its finally block — freeing that copy — and only afterwards reads buf+off to build the ExtraData payload. ExtraData.extra is therefore filled from freed memory.
This is the same class of bug as #677, on a code path that fix did not cover.
Reproducer (from #720)
Reproduced on macOS 15 / arm64 / CPython 3.14 with 1.2.1 and with current main. Running the same reproducer under the macOS allocator's free-poisoning makes the use-after-free unambiguous:
so arbitrary heap contents can end up in ExtraData.extra, not merely a shifted copy of the input. The reporter's ASan output (memcpy-param-overlap) is the same event seen from the other side: PyBytes_FromStringAndSize re-allocates the just-freed block and copies it onto itself.
Unpacker.feed() uses the same helper but copies via append_buffer() before releasing, so it is unaffected. fallback.py is also unaffected.
Fix
Copy the extra data out before releasing the view. Kept minimal: unpack_data() still runs outside the try, so only the PyBytes_FromStringAndSize call moved.
Test
test_unpack_noncontiguous_memoryview_extra_data in test/test_memoryview.py, next to the #677 test. Verified it fails on main (At index 4 diff: b'\x00' != b'a') and passes with the fix.
Full suite passes for both implementations — 135 passed (Cython), 133 passed / 2 skipped (MSGPACK_PUREPYTHON=1) — and also with MallocScribble=1.
Note that whether the corruption is observable depends on allocator reuse, so the test uses a payload/extra size combination that reliably triggers it on CPython's pymalloc; the assertion itself is unconditionally correct.