Klocwork code scan critical error warning about HGOTO_ERROR, is there a small change we can make to avoid this?

[Ben136KBC]

We are required to run a code scanning tool and it really does not like HGOTO_ERROR. Can someone perhaps please look at what is below and advise? Maybe there is a simple check that can be put in the code somewhere to appease the tool? This is based on "HDF5 version 1.12.1 released on 2021-07-01", a little old, but the relevant code seems unchanged.

The critical error is: Buffer Overflow - Local Array Index Out of Bounds

Details: Array 'H5I_type_info_array_g' of size 127 may use index value(s) 0..127

Occurs in many places like: H5Eint.c with lines like:

HGOTO_ERROR(H5E_ERROR, H5E_CANTDEC, FAIL, "unable to decrement ref count on error message")

Traceback:

H5Eint.c:822: 'H5E_ERROR_g' is passed as argument '$6' to function 'H5E_printf_stack'.
H5Eint.c:822: Index value(s) 0..127 may be used to access array of size 127 while calling 'H5E_printf_stack'.

So somewhere it could possibly use a value of 127 to access the array when the array is only 0 through 126?

But I don't see exactly where the actual error would happen. Note that H5Ipkg.h has lines like this:

#define TYPE_BITS 7
#define TYPE_MASK (((hid_t)1 << TYPE_BITS) - 1) <<===== This is 127
#define H5I_MAX_NUM_TYPES TYPE_MASK
H5_DLLVAR H5I_type_info_t *H5I_type_info_array_g[H5I_MAX_NUM_TYPES];

But I can't see where the H5E_printf_stack or something it calls accesses H5I_type_info_array_g. Maybe in H5Itype_exists?

And I do see:

int H5I_next_type_g = (int)H5I_NTYPES;

And the next_type could be used as an index in some place, so this is perhaps not quite as safe and clean.... Maybe that?!?

Also I do see: H5E_ERROR_g gets set to -1 and if you & that with TYPE_MASK you would get 127?

So there is smoke here but I can't see a fire.

[jhendersonHDF]

Hi @Ben136KBC,

I'm going to guess that the scanning tool is picking up cases where the H5I_next_type_g variable might be used to index into H5I_type_info_array_g after it has been incremented to be equal to H5I_MAX_NUM_TYPES. For example, if an application registers types with https://github.com/HDFGroup/hdf5/blob/develop/src/H5I.c#L92, eventually, at https://github.com/HDFGroup/hdf5/blob/develop/src/H5Iint.c#L180-L183, H5I_next_type_g will be incremented so that it is H5I_MAX_NUM_TYPES. While this should prevent registering additional types, there are places in the library where it could potentially continue being used for indexing into H5I_type_info_array_g. Most likely these are some places in the library where a check that the index is within the array bounds is only in an assertion and not a real error check for production builds, such as https://github.com/HDFGroup/hdf5/blob/develop/src/H5Iint.c#L813, https://github.com/HDFGroup/hdf5/blob/develop/src/H5Iint.c#L848 and https://github.com/HDFGroup/hdf5/blob/develop/src/H5Iint.c#L1433.