[MEDIUM] Fix cross-buffer buffer-pool use-after-free - #38
Open
OskarEichler wants to merge 1 commit into
Open
Conversation
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.
Summary
The buffer pool can release a memory page while a live
CBOR::Bufferstill reads from it. A second buffer that reuses the page can overwrite the first buffer's unread bytes, producing deterministic cross-buffer data disclosure and corruption. This fixes the use-after-free by transferring read-memory ownership when the inline tail becomes a heap chunk and clearing stale ownership pointers before destruction.This addresses #35 and ports the corresponding ownership repair from msgpack 1.8.4.
Reproduction and mechanism
The complete deterministic reproducer is in #35. On cbor 0.5.10.3 with Ruby 3.4.7/aarch64 Linux it returned bytes written by a different buffer in 50/50 runs. Removing the second buffer or using only copied writes produced 0/50 corruptions.
_msgpack_buffer_add_new_chunkcopiesb->tailinto a new chunk without movingb->rmem_owner. When the old head is shifted, the page is returned to the process-global pool even though the new tail still references it. Another buffer can then reuse and overwrite the page.Verification
No tests were changed because this repository asks contributors not to add or modify tests.
Scope and limitations
The demonstrated path uses the low-level
CBOR::BufferAPI with a specific mix of copied and referenced writes across buffers. I could not reach it throughCBOR.decodeor a singleCBOR::Unpacker; those paths use buffer thresholds that avoid the triggering ownership transition.The reproducer was exercised on aarch64 Linux. The ownership defect is platform-independent C logic, but allocator behavior on other platforms was not measured.
Breaking changes
None. The patch only corrects internal buffer ownership and preserves the public API and encoded data.