Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add buffer overrun checks to H5O__layout_decode and H5O__sdspace_decode #2679

Merged
merged 2 commits into from
Apr 11, 2023

Conversation

jhendersonHDF
Copy link
Collaborator

No description provided.

@jhendersonHDF jhendersonHDF added Merge - To 1.12 Priority - 1. High 🔼 These are important issues that should be resolved in the next release Component - C Library Core C library issues (usually in the src directory) Type - Bug / Bugfix Please report security issues to [email protected] instead of creating an issue on GitHub labels Apr 6, 2023
@jhendersonHDF jhendersonHDF force-pushed the layout_space_decode_checks branch from 70c5d53 to c17ca84 Compare April 6, 2023 02:39
/* Check if a read of size bytes starting at ptr would overflow past
* the last valid byte, pointed to by buffer_end.
*/
#define H5_IS_BUFFER_OVERFLOW(ptr, size, buffer_end) (((ptr) + (size)-1) > (buffer_end))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Borrowing this until we determine final placement..

Copy link
Contributor

@qkoziol qkoziol left a comment

Choose a reason for hiding this comment

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

Suggest H5E_NOSPACE -> H5E_OVERFLOW

@jhendersonHDF
Copy link
Collaborator Author

Suggest H5E_NOSPACE -> H5E_OVERFLOW

"Address overflowed" seems a bit awkward to me as well to describe the issue that occurred, but I don't have any strong opinion either way. I can change it.

@jhendersonHDF jhendersonHDF force-pushed the layout_space_decode_checks branch 2 times, most recently from 21b9dd7 to 47bb446 Compare April 7, 2023 20:22
src/H5Olayout.c Outdated
@@ -164,29 +180,46 @@ H5O__layout_decode(H5F_t *f, H5O_t H5_ATTR_UNUSED *open_oh, unsigned H5_ATTR_UNU
* size in the dataset code, where we've got the dataspace
* information available also. - QAK 5/26/04
*/
if (H5_IS_BUFFER_OVERFLOW(p, (ndims * sizeof(uint32_t)), p_end))
HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding")
p += ndims * 4; /* Skip over dimension sizes (32-bit quantities) */
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this '4' be replaced with a size call to match the overflow check?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure. Changed.

@jhendersonHDF jhendersonHDF force-pushed the layout_space_decode_checks branch from 47bb446 to df97bbc Compare April 11, 2023 02:45
@lrknox lrknox merged commit bc8fa3a into HDFGroup:develop Apr 11, 2023
brtnfld pushed a commit to brtnfld/hdf5 that referenced this pull request Apr 13, 2023
jhendersonHDF added a commit to jhendersonHDF/hdf5 that referenced this pull request Apr 13, 2023
jhendersonHDF added a commit to jhendersonHDF/hdf5 that referenced this pull request Apr 13, 2023
jhendersonHDF added a commit to jhendersonHDF/hdf5 that referenced this pull request Apr 13, 2023
byrnHDF pushed a commit to byrnHDF/hdf5 that referenced this pull request Apr 16, 2023
@jhendersonHDF jhendersonHDF deleted the layout_space_decode_checks branch April 17, 2023 20:02
brtnfld pushed a commit to brtnfld/hdf5 that referenced this pull request May 17, 2023
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) Priority - 1. High 🔼 These are important issues that should be resolved in the next release Type - Bug / Bugfix Please report security issues to [email protected] instead of creating an issue on GitHub
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants