Skip to content

reject SOHM list message count exceeding list_max#6499

Open
naruto-lgtm wants to merge 3 commits into
HDFGroup:developfrom
naruto-lgtm:sohm-list-count-bound
Open

reject SOHM list message count exceeding list_max#6499
naruto-lgtm wants to merge 3 commits into
HDFGroup:developfrom
naruto-lgtm:sohm-list-count-bound

Conversation

@naruto-lgtm

@naruto-lgtm naruto-lgtm commented Jul 1, 2026

Copy link
Copy Markdown

Describe your changes

When a shared object header message index is stored as a list, both H5SM__cache_list_verify_chksum() and H5SM__cache_list_deserialize() take the num_messages count straight from the on-disk index header. The image buffer and the list->messages array are both sized for list_max entries, so a file whose num_messages is larger than list_max makes verify_chksum size the checksum region past the buffer and makes the deserialize loop write past the array and read past the buffer. A list holds at most list_max messages on disk before it is promoted to a B-tree, so any file that violates that is corrupt.

Reject num_messages > list_max at both callbacks before the count is used to size a read or index the array. That header field is the trust boundary here, and the rest of the SOHM code already assumes the count fits within list_max, so validating it once at load time keeps every later list->messages access sound.

Issue ticket number (GitHub or JIRA)

#6501

Checklist before requesting a review

  • My code conforms to the guidelines in CONTRIBUTING.md
  • I made an entry in release_docs/CHANGELOG.md (bug fixes, new features)
  • I added a test (bug fixes, new features)

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Review Checklist

This PR touches the following areas. Each needs a sign-off
from its listed owners before merging.

hyoklee
hyoklee previously approved these changes Jul 2, 2026

@hyoklee hyoklee left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

release_docs looks good.

@lrknox lrknox self-assigned this Jul 2, 2026
@lrknox lrknox added the Component - C Library Core C library issues (usually in the src directory) label Jul 2, 2026

@lrknox lrknox left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A test should be included if possible.

@github-project-automation github-project-automation Bot moved this from To be triaged to In progress in HDF5 - TRIAGE & TRACK Jul 2, 2026
mattjala
mattjala previously approved these changes Jul 2, 2026
Create a file with a shared-message list index, corrupt the on-disk
message count so it exceeds list_max (repairing the table checksum),
and confirm reopening rejects the file instead of overrunning the
list image buffer and message array.
@naruto-lgtm naruto-lgtm dismissed stale reviews from mattjala and hyoklee via d53b8b1 July 9, 2026 08:03
@naruto-lgtm

Copy link
Copy Markdown
Author

Added a test in tsohm.c (test_sohm_reject_bad_count). It creates a file with a list-index SOHM, pushes the on-disk message count past list_max and repairs the table checksum, then reopens and shares a message so the list gets protected. With the fix that load is rejected cleanly; on the pre-fix code the same path is the heap-buffer-overflow read in H5F_get_checksums via H5SM__cache_list_verify_chksum, so it trips under ASan. It's guarded to the default sec2 driver since it edits the file image directly.

@github-actions github-actions Bot requested a review from mattjala July 9, 2026 08:03
@github-actions github-actions Bot requested a review from hyoklee July 9, 2026 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component - C Library Core C library issues (usually in the src directory)

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

5 participants