Skip to content

Avoid out-of-bounds dims read for a scalar HDF5 dataspace - #319

Open
naruto-lgtm wants to merge 1 commit into
tbeu:masterfrom
naruto-lgtm:mat73-scalar-dataspace-dims
Open

Avoid out-of-bounds dims read for a scalar HDF5 dataspace#319
naruto-lgtm wants to merge 1 commit into
tbeu:masterfrom
naruto-lgtm:mat73-scalar-dataspace-dims

Conversation

@naruto-lgtm

Copy link
Copy Markdown
Contributor

Repro: open a v7.3 file with a numeric dataset stored as a scalar (rank-0) HDF5 dataspace and marked MATLAB_empty, or a sparse variable whose ir/jc/data subdataset is stored the same way.
Cause: Mat_H5ReadDims sizes its result with malloc(*rank * sizeof(size_t)), so a scalar dataspace gives malloc(0); Mat_H5ReadDatasetInfo's empty branch and the sparse ir/jc/data readers in Mat_VarRead73 then read dims[0] off that zero-length buffer (ASan heap-buffer-overflow read at mat73.c:920 and :3204).
Fix: return one zeroed element for a scalar dataspace so dims[0] stays in bounds. rank stays 0, so the struct-field reader that already special-cases a scalar is unchanged and the malformed empty/sparse datasets are rejected downstream.

@naruto-lgtm

Copy link
Copy Markdown
Contributor Author

any thoughts here?

Comment thread src/mat73.c
* zero-length buffer, so they read out of bounds. Return one zeroed
* element instead; rank stays 0 so callers that special-case a scalar
* still do, and those reading dims[0] see a defined value. */
perm_dims = (size_t *)calloc(1, sizeof(*perm_dims));

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

What if calloc fails, i.e. returns NULL?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch, that path was sloppy. Fixed: *nelems is only set on success and the failure now emits the same Mat_Critical("Error allocating memory for matvar->dims") as the other allocation failures in this function, so it behaves like them instead of returning NULL with nelems already bumped to 1.

The NULL return itself is safe at every call site, I checked all seven (916, 1093, 1214, 3212, 3269, 3334) and they all guard on it. rank is already 0 on this path, so callers see the same shape as the existing failure branches.

Squashed it into the one commit.

@naruto-lgtm
naruto-lgtm force-pushed the mat73-scalar-dataspace-dims branch from f564470 to cef8bbb Compare August 4, 2026 18:18
@tbeu
tbeu force-pushed the mat73-scalar-dataspace-dims branch from cef8bbb to b565dc1 Compare August 11, 2026 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants