Conversation
Signed-off-by: Robert McConnell <robert@mcc0nnell.org>
Signed-off-by: Robert McConnell <robert@mcc0nnell.org>
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
Preserve caller input that aliases
ogg_stream_state.body_datawhen framing APIs compact or grow the internal body buffer. The affected public entry points areogg_stream_iovecin()/ogg_stream_packetin()on the encoding side andogg_stream_pagein()on the page-decomposition side.Problem
All three paths can retain a caller pointer into
os->body_data, mutate or grow that allocation, and only then consume the saved pointer. If_os_body_expand()moves the allocation, the latermemcpy()reads freed storage.Two standalone sanitizer repros reliably fail on current
main(06a5e026):ogg_stream_packetin()with a 16 KiB packet aliasing the stream's initial body buffer: ASan heap-use-after-free, 16 KiB read inogg_stream_iovecin(), after_os_body_expand()reallocates the source allocation.ogg_stream_pagein()with a valid-sized lacing table andog.bodyaliasing the same 16 KiB allocation: ASan heap-use-after-free, 16 KiB read after_os_body_expand()moves the body buffer.ogg_stream_iovecin()directly reproduces the first failure as well.Fix
When submitted packet/page data points into the stream's current body allocation, snapshot that input before compaction or growth. External caller buffers remain on the existing path and incur no extra allocation.
Validation
test_framingunder Clang ASan + UBSan: passogg_stream_iovecin()self-alias reproducer: ASan UAF before, byte-identical success afterogg_stream_packetin()wrapper reproducer: ASan UAF before, byte-identical success afterogg_stream_pagein()body-alias reproducer: ASan UAF before, byte-identical success after-pedantic-errors -Wall -Wextra -Werror: passgit diff --check: passThis is the framing-buffer analogue of the self-referential growth problem addressed separately in bit-packing PR #106; the affected storage and APIs are independent.
Signed-off-by: Robert McConnell robert@mcc0nnell.org