Validate VL datatype type during decode and check file pointer in H5T_set_loc#6395
Open
tbeu wants to merge 1 commit into
Open
Validate VL datatype type during decode and check file pointer in H5T_set_loc#6395tbeu wants to merge 1 commit into
tbeu wants to merge 1 commit into
Conversation
tbeu
requested review from
derobins,
fortnern,
jhendersonHDF and
lrknox
as code owners
tbeu
requested review from
bmribler,
brtnfld,
glennsong09,
mattjala,
qkoziol and
vchoi-hdfgroup
as code owners
mattjala
approved these changes
May 4, 2026
tbeu
force-pushed
the
fix/vlen-invalid-type-decode
branch
2 times, most recently
from
1d856f9 to
54495aa
Compare
bmribler
approved these changes
May 8, 2026
jhendersonHDF
approved these changes
May 12, 2026
tbeu
force-pushed
the
fix/vlen-invalid-type-decode
branch
from
54495aa to
ff21ec4
Compare
…_set_loc H5O__dtype_decode_helper() reads vlen.type from the file without validation. With corrupted HDF5 files (e.g. from fuzzing), this field can have an invalid value that is neither H5T_VLEN_SEQUENCE nor H5T_VLEN_STRING, which later triggers assert(0) in H5T__vlen_set_loc() (debug builds) or a NULL pointer dereference / SEGV in release builds. Fix by: 1. Adding a validation check in H5O__dtype_decode_helper() immediately after reading the vlen.type field, returning an error if the value is invalid. 2. Adding a NULL file pointer check in H5T_set_loc() before calling H5T__vlen_set_loc() when loc == H5T_LOC_DISK, so the low-level assert(file) invariant is never violated. This fixes the root cause at the decode level where the bad value enters the system, as requested in review of HDFGroup#6378 and HDFGroup#6385. Closes HDFGroup#6378 Closes HDFGroup#6385 Found by OSS-Fuzz via the matio fuzzer (ClusterFuzz testcase 5366895365914624).
tbeu
force-pushed
the
fix/vlen-invalid-type-decode
branch
from
ff21ec4 to
d495a3d
Compare
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.
Description
This fixes the crash reported in #6378 and #6385 by addressing the root cause at the decode level, as requested by reviewers.
H5O__dtype_decode_helper()readsvlen.typefrom the file without validation. With corrupted HDF5 files (e.g. from fuzzing), this field can have an invalid value that is neitherH5T_VLEN_SEQUENCEnorH5T_VLEN_STRING, which later triggersassert(0)inH5T__vlen_set_loc()(debug builds) or a NULL pointer dereference / SEGV in release builds.Changes
src/H5Odtype.c (
H5O__dtype_decode_helper): Added validation immediately after readingvlen.typefrom the file. If the value is invalid,HGOTO_ERRORis returned so the corrupted type is rejected at the point where the bad value enters the system.src/H5T.c (
H5T_set_loc): Added a NULL file pointer check before callingH5T__vlen_set_loc()whenloc == H5T_LOC_DISK, so the low-levelassert(file)invariant is never violated even if a higher-level caller fails to provide a valid file.Rationale
Per the review feedback on #6378 and #6385: the
assert()statements inH5T__vlen_set_loc()and the vlen disk operations are intended to catch internal programming errors and should remain. The proper fix is to validate inputs at the higher level where corrupted data is first ingested. This PR does exactly that.How was this found
Found by OSS-Fuzz via the matio fuzzer (ClusterFuzz testcase 5366895365914624).
OSS-Fuzz issue: https://issues.oss-fuzz.com/issues/472641758
Reproducer file: clusterfuzz-testcase-minimized-matio_fuzzer-5366895365914624.zip
Supersedes #6378 and #6385.