check object header message size after decoding its header - #6589
Open
naruto-lgtm wants to merge 4 commits into
Open
check object header message size after decoding its header#6589naruto-lgtm wants to merge 4 commits into
naruto-lgtm wants to merge 4 commits into
Conversation
naruto-lgtm
requested review from
fortnern,
glennsong09,
hyoklee,
jhendersonHDF,
lrknox,
mattjala and
vchoi-hdfgroup
as code owners
August 3, 2026 09:38
github-actions
Bot
removed request for
fortnern,
glennsong09,
jhendersonHDF,
lrknox and
mattjala
August 3, 2026 09:44
Contributor
Review ChecklistThis PR touches the following areas. Each needs a sign-off
|
hyoklee
previously approved these changes
Aug 4, 2026
|
|
||
| ### Fixed a heap buffer overflow when decoding object header messages | ||
|
|
||
| The size stored in an object header message header was checked against the chunk before the rest of that message header was decoded, so the check allowed a message body to start up to four bytes further into the chunk than the check accounted for. A corrupted or fuzzed file could declare a size that passed the check and still ran past the end of the chunk image, and the message's decode callback was then handed a buffer end outside the allocation. `H5O__chunk_deserialize()` now checks the message size once the whole message header has been decoded. |
Member
There was a problem hiding this comment.
so the check allowed -> allowing
Contributor
Author
There was a problem hiding this comment.
Done, reworded to "allowing".
vchoi-hdfgroup
previously approved these changes
Aug 4, 2026
hyoklee
previously approved these changes
Aug 4, 2026
hyoklee
previously approved these changes
Aug 6, 2026
|
|
||
| ### Fixed a heap buffer overflow when decoding object header messages | ||
|
|
||
| The size stored in an object header message header was checked against the chunk before the rest of that message header was decoded, allowing a message body to start up to four bytes further into the chunk than the check accounted for. A corrupted or fuzzed file could declare a size that passed the check and still ran past the end of the chunk image, and the message's decode callback was then handed a buffer end outside the allocation. `H5O__chunk_deserialize()` now checks the message size once the whole message header has been decoded. |
Contributor
Author
There was a problem hiding this comment.
Done, switched to extended.
Signed-off-by: Nayyar <nayyar@bugqore.com>
hyoklee
approved these changes
Aug 10, 2026
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
5 participants
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.
Describe your changes
Repro: open a file whose object header holds a message that declares a body reaching the end of its chunk, where the tail of the message header sits in those last bytes. Issue #6401 has a fuzzer PoC for it. By hand it is a version 1 header with a 12 byte chunk 0, one mtime message declaring an 8 byte body, and only 4 bytes left after the 8 byte message header.
Cause:
H5O__chunk_deserialize()comparesmesg_sizeagainstp_endwhilechunk_imagestill points at the flags byte, so the flags plus reserved bytes (version 1) or flags plus creation index (version 2) that follow are not counted. The body begins up to four bytes further into the chunk than the check assumed, andmesg->raw/mesg->raw_sizethen describe a region ending past the chunk image.Fix: run the same check once the whole message header has been decoded, against the pointer that actually becomes the message body.
Every message class builds its own
p_endfromrawandraw_size, so this is not specific to mtime. Whichever decoder the type byte selects gets handed a buffer end outside the allocation, which is why the check belongs in the chunk loop rather than in the individual decoders, where the chunk image bound is not known.On the unpatched library ASan reports a heap-buffer-overflow read at
H5Omtime.c:137fromH5Ocache.c:1475, 0 bytes past the chunk image, and is clean afterwards. The trailing image size check rejects the file either way, so the new case intest/ohdr.cearns its keep under a sanitizer or valgrind, the same waytest_ohdr_badness()above it does.Issue ticket number (GitHub or JIRA)
#6401
Checklist before requesting a review