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.

Sign up for GitHub

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

Jump to bottom

Add check for decoded datatype precision overflow #4315

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from
Draft

Add check for decoded datatype precision overflow #4315

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/H5Odtype.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,8 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t
/* Check for invalid datatype size */
if (dt->shared->size == 0)
HGOTO_ERROR(H5E_OHDR, H5E_BADVALUE, FAIL, "invalid datatype size");
if (H5T_IS_ATOMIC(dt->shared) && (dt->shared->size > SIZE_MAX / 8))
HGOTO_ERROR(H5E_OHDR, H5E_BADVALUE, FAIL, "number of bits in atomic datatype would overflow size_t");

switch (dt->shared->type) {
case H5T_INTEGER:
Expand Down
13 changes: 7 additions & 6 deletions src/H5T.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6261,10 +6261,11 @@ H5T_set_loc(H5T_t *dt, H5VL_object_t *file, H5T_loc_t loc)
/* Check the datatype of this element */
switch (dt->shared->type) {
case H5T_ARRAY: /* Recurse on VL, compound and array base element type */
/* Recurse if it's VL, compound, enum or array */
/* Recurse if it's VL, compound, enum, array or reference */
/* (If the force_conv flag is _not_ set, the type cannot change in size, so don't recurse) */
if (dt->shared->parent->shared->force_conv &&
H5T_IS_COMPLEX(dt->shared->parent->shared->type)) {
(H5T_IS_COMPLEX(dt->shared->parent->shared->type) ||
H5T_IS_REF(dt->shared->parent->shared))) {
/* Keep the old base element size for later */
old_size = dt->shared->parent->shared->size;

Expand Down Expand Up @@ -6302,10 +6303,11 @@ H5T_set_loc(H5T_t *dt, H5VL_object_t *file, H5T_loc_t loc)
/* Set the member type pointer (for convenience) */
memb_type = dt->shared->u.compnd.memb[i].type;

/* Recurse if it's VL, compound, enum or array */
/* Recurse if it's VL, compound, enum, array or reference */
/* (If the force_conv flag is _not_ set, the type cannot change in size, so don't recurse)
*/
if (memb_type->shared->force_conv && H5T_IS_COMPLEX(memb_type->shared->type)) {
if (memb_type->shared->force_conv &&
(H5T_IS_COMPLEX(memb_type->shared->type) || H5T_IS_REF(memb_type->shared))) {
/* Keep the old field size for later */
old_size = memb_type->shared->size;

Expand Down Expand Up @@ -6347,8 +6349,7 @@ H5T_set_loc(H5T_t *dt, H5VL_object_t *file, H5T_loc_t loc)
* them as part of the same blob)*/
/* (If the force_conv flag is _not_ set, the type cannot change in size, so don't recurse) */
if (dt->shared->parent->shared->force_conv &&
H5T_IS_COMPLEX(dt->shared->parent->shared->type) &&
(dt->shared->parent->shared->type != H5T_REFERENCE)) {
H5T_IS_COMPLEX(dt->shared->parent->shared->type)) {
Copy link
Member

Choose a reason for hiding this comment

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

Weird indenting

if ((changed = H5T_set_loc(dt->shared->parent, file, loc)) < 0)
HGOTO_ERROR(H5E_DATATYPE, H5E_CANTINIT, FAIL, "Unable to set VL location");
if (changed > 0)
Expand Down
6 changes: 4 additions & 2 deletions src/H5Tpkg.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@
#define H5T_NAMELEN 32

/* Macro to ease detecting "complex" datatypes (i.e. those with base types or fields) */
#define H5T_IS_COMPLEX(t) \
((t) == H5T_COMPOUND || (t) == H5T_ENUM || (t) == H5T_VLEN || (t) == H5T_ARRAY || (t) == H5T_REFERENCE)
#define H5T_IS_COMPLEX(t) ((t) == H5T_COMPOUND || (t) == H5T_ENUM || (t) == H5T_VLEN || (t) == H5T_ARRAY)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I believe H5T_REFERENCE was only added to this macro to make the logic in H5T_set_loc work correctly for reference types. Reference types are treated as atomic types, but would be excluded when checking a datatype with the H5T_IS_ATOMIC macro due to the !H5T_IS_COMPLEX check.


/* Macro to ease detecting fixed "string" datatypes */
#define H5T_IS_FIXED_STRING(dt) (H5T_STRING == (dt)->type)
Expand All @@ -57,6 +56,9 @@
/* Macro to ease detecting fixed or variable-length "string" datatypes */
#define H5T_IS_STRING(dt) (H5T_IS_FIXED_STRING(dt) || H5T_IS_VL_STRING(dt))

/* Macro to ease detecting reference datatypes */
#define H5T_IS_REF(dt) (H5T_REFERENCE == (dt)->type)

/* Macro to ease detecting atomic datatypes */
#define H5T_IS_ATOMIC(dt) (!(H5T_IS_COMPLEX((dt)->type) || (dt)->type == H5T_OPAQUE))

Expand Down
Loading