File format security issues #4234

derobins · 2024-03-25T06:52:43Z

Addresses file format security issues detected via fuzzing.

Credit: Amazon Web Services and @qkoziol

NOTE: Requires fixup! * Sizes are tweaked by one byte for unknown reasons * tmisc_38 still barfs

…y_fixes

src/H5encode.h

check value against PTRDIFF_MAX before casting to ptrdiff_t update H5_IS_KNOWN_BUFFER_OVERFLOW macro to use H5_IS_BUFFER_OVERFLOW

jhendersonHDF · 2024-03-26T21:14:08Z

CMake seems to be failing in the H5DUMP-tnbit test because the compression ratio line isn't getting masked out for some reason. Not sure on netcdf failures, but investigating.

jhendersonHDF · 2024-03-26T21:53:07Z

Updating the H5_IS_KNOWN_BUFFER_OVERFLOW macro to use H5_IS_BUFFER_OVERFLOW when skip is false caused the netCDF tests to start failing in one test case when trying to open a file after creating a dataset with an opaque type in it. This fails both before and after my changes to H5_IS_BUFFER_OVERFLOW. More investigation is likely needed there, but I'll put the change back for now.

fortnern · 2024-03-26T21:58:32Z

src/H5Odtype.c

+            /* Sanity checks */
+            if (dt->shared->u.atomic.offset >= (dt->shared->size * 8))
+                HGOTO_ERROR(H5E_DATATYPE, H5E_BADRANGE, FAIL, "integer offset out of bounds");
+            if (0 == dt->shared->u.atomic.prec)


Could we support this as a no-op?

fortnern · 2024-03-26T22:04:56Z

src/H5Odtype.c

@@ -452,6 +487,13 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t
                }
                if (temp_type->shared->size == 0)
                    HGOTO_ERROR(H5E_DATATYPE, H5E_CANTDECODE, FAIL, "type size can't be zero");
+                if ((dt->shared->u.compnd.memb[dt->shared->u.compnd.nmembs].offset +


I think we should either always use offset + size > dt->shared->size or offset + size - 1 >= dt->shared->size

src/H5Oattribute.c

derobins · 2024-03-26T22:29:15Z

CMake seems to be failing in the H5DUMP-tnbit test because the compression ratio line isn't getting masked out for some reason. Not sure on netcdf failures, but investigating.

In the Autotools, I had to change the lead digit of a macro, since it goes from 10 to 1.

jhendersonHDF · 2024-03-27T20:15:24Z

Will address the H5_IS_BUFFER_OVERFLOW issue and @fortnern's comments in a separate PR so as to not hold up this PR and create a bunch of extra testing work.

* Take user block into account when returning chunk addresses (#4236) Both H5Dchunk_iter() and H5Dget_chunk_info(_by_coord)() did not take the size of the user block into account when reporting addresses. Since the #1 use of these functions is to root around in the file for the raw data, this is kind of a problem. Fixes GitHub issue #3003 * Fix a minor warning in h5test.c (#4242) * Turn on -Werror for Java in GitHub -Werror workflows (#4243) * Update Windows CI to not install ninja (#4230) * Rework Fortran macros to use the proper code. (#4240) * Correct reference copy for 16 API (#4244) * Determine MPI LOGICAL during build, used in tests. (#4246) * Skip userblock test in chunk_info.c for multi-file VFDs (#4249) * Match generators with real cmake -G output on Windows (#4252) * Add Julia GitHub Actions. (#4123) * Re-revert to using autoreconf in autogen.sh (#4253) We previously tried removing the per-tool invocation of the Autotools and instead simply invoked autoreconf (PR #1906). This was reverted when it turned out that the NAG Fortran compiler had trouble with an undecorated -shared linker flag. It turns out that this is due to a bug in libtool 2.4.2 and earlier. Since this version of libtool is over a decade old, we're un-reverting the change. We've added a release note for anyone who has to build from source on elderly platforms. Fixes #1343 * Rewrite H5T__path_find_real for clarity (#4225) * Move conversion path free logic to helper function * Add tgz extensions on names (#4255) * Remove an error check regarding large cache objects (#4254) * Remove an error check regarding large cache objects In PR#4231 an assert() call was converted to a normal HDF5 error check. It turns out that the original assert() was added by a developer as a way of being alerted that large cache objects existed instead of as a guard against incorrect behavior, making it unnecessary in either debug or release builds. The error check has been removed. * Update RELEASE.txt * File format security issues (#4234) * Add job timeout to cygwin workflow (#4260) * Replace user-define with user-defined (#4261) * Improve the CMake clang -fsanitize=memory flags (#4267) -fsanitize=memory is almost useless without using -fsanitize-memory-track-origins=2 and we shoud probably add -fno-optimize-sibling-calls as well. * Add documentation (H5M) (#4259) * Add documentation (H5P) (#4262) * MPI type correction (#4268) * corrected type for MPI_*_f2c APIs * fixed return type of callback * reset compilation flags of logical test program * Clean up test/cmpd_dtransform.c (#4270) * Clean up test/cmpd_dtransform.c * Fix uninitialized memory warning from sanitizers * FAIL_STACK_ERROR --> TEST_ERROR * Emit output * Delete test file when done * Fix typo * H5Fdelete() --> remove() * Fix uninitialized memory issues in packet table (#4271) * replace deprecated CMAKE_COMPILER_IS_GNU** (#4272) * Prevent stack overflows in H5E__push_stack (#4264) * Minor fixes after merge of file format security fixes (#4263) * Update H5_IS_BUFFER_OVERFLOW to account for 'size' of 0 * Invert a few checks to avoid function call * CHECK --> CHECK_PTR in tmisc.c (#4274) * Add release note for CVE-2017-17507 (#4275) * Update Cygwin installation guide (#4265) * Addresses configuration fortran testing flags (#4276) * turn warnings to errors in fortran configure test * Intel fortran test fix * Merge julia workflows into standard ci format (#4273) * Fix range check in H5_addr_overlap (#4278) When the H5_addr_overlap macro was updated to use H5_RANGE_OVERLAP, it failed to take into account that H5_RANGE_OVERLAP expects the range to be inclusive. This lead to an assertion failure in H5MM_memcpy due to a memcpy operation on overlapping memory. This has been fixed by subtracting 1 from the calculated high bound values passed to H5_RANGE_OVERLAP * Fix potential buffer read overflows in H5PB_read (#4279) H5PB_read previously did not account for the fact that the size of the read it's performing could overflow the page buffer pointer, depending on the calculated offset for the read. This has been fixed by adjusting the size of the read if it's determined that it would overflow the page.

derobins added 28 commits March 22, 2024 09:06

Patch H5encode.h

82bfbc3

Patched H5Zscaleoffset.c

f56fc3b

H5O patches (partial)

a4bf93a

Patch H5Pencdec.c and related

ee08a41

Patch H5HG.c

a95e320

Patch H5Rint.c

cd8efec

Format source

d6a2c9d

Add H5F RFIC boilerplate

6eceee0

Add H5P RFIC code

f6ae41b

Add H5T_is_numeric_with_unusual_unused_bits()

1110434

Add H5_RANGE_OVERLAP()

cdd8453

Patch H5S

f6549ba

Patch H5Olinfo.c

561af7c

Patch H5A and related

3e3bbd8

Patch H5Dio.c

4f1138f

Format source

8bf4c65

Patch h5tools_str.c

10a6703

Patch H5Odtype.c

3f8fc7d

Patch H5Tcommit.c

74a0cb1

Patch tmisc.c test 22

825dff5

Add unused bits checks and tools patches

e3a7763

NOTE: Requires fixup! * Sizes are tweaked by one byte for unknown reasons * tmisc_38 still barfs

Patch H5Tconv.c

6353d9e

Format source

8752758

Update overflow macro

e4f5a06

Merge remote-tracking branch 'canonical/develop' into aws_13m_securit…

5fc9fd0

…y_fixes

Fix TRACE line

7d9f5e5

Add tmisc38 binary files & enable test

3db7d84

Fix spelling

44b78d0

derobins added Merge - To 1.14 Priority - 0. Blocker ⛔ This MUST be merged for the release to happen labels Mar 25, 2024

derobins and others added 11 commits March 26, 2024 07:54

New functions since 1.14.4 in Doxygen

4138872

Mask out nbit h5dump test

0987047

vlen const changes in H5T

172fb2d

Reference --> vlen in a comment

c884dc1

ID cleanup in tmisc38

6deadd2

Fix tmisc38 filename C

636bf02

Merge remote-tracking branch 'canonical/develop' into aws_13m_securit…

b1f9707

…y_fixes

Ensure ref. priv. buffer pointers are aligned

4c0399f

Handle H5P_DEFAULT in H5Pget_relax_file_integrity_checks

a452379

Handle IBM long double type in H5T_is_numeric_with_unusual_unused_bits

2aa3bec

Suppress type-limits warnings for Clang as well

19056d9

jhendersonHDF reviewed Mar 26, 2024

View reviewed changes

src/H5encode.h Show resolved Hide resolved

jhendersonHDF and others added 4 commits March 26, 2024 15:33

Address warning in H5Olayout.c

b495772

Rewrite H5_IS_BUFFER_OVERFLOW slightly

c1b45c3

check value against PTRDIFF_MAX before casting to ptrdiff_t update H5_IS_KNOWN_BUFFER_OVERFLOW macro to use H5_IS_BUFFER_OVERFLOW

Committing clang-format changes

9bd14a3

Initialize var.

819c3cd

Revert change to H5_IS_KNOWN_BUFFER_OVERFLOW

df99d07

fortnern reviewed Mar 26, 2024

View reviewed changes

src/H5Oattribute.c Show resolved Hide resolved

Mask out compression filter in CMake for tnbit test

42d4015

derobins merged commit 136739b into HDFGroup:develop Mar 27, 2024
52 checks passed

derobins mentioned this pull request Mar 28, 2024

Buffer Overflow in H5A__close at H5Aint.c #2649

Closed

jhendersonHDF mentioned this pull request Mar 29, 2024

Fix range check in H5_addr_overlap #4278

Merged

lrknox pushed a commit to lrknox/hdf5 that referenced this pull request Mar 29, 2024

File format security issues (HDFGroup#4234)

95ae39e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

File format security issues #4234

File format security issues #4234

derobins commented Mar 25, 2024

jhendersonHDF commented Mar 26, 2024

jhendersonHDF commented Mar 26, 2024

fortnern Mar 26, 2024

fortnern Mar 26, 2024 •

edited

Loading

derobins commented Mar 26, 2024

jhendersonHDF commented Mar 27, 2024

File format security issues #4234

File format security issues #4234

Conversation

derobins commented Mar 25, 2024

jhendersonHDF commented Mar 26, 2024

jhendersonHDF commented Mar 26, 2024

fortnern Mar 26, 2024

Choose a reason for hiding this comment

fortnern Mar 26, 2024 • edited Loading

Choose a reason for hiding this comment

derobins commented Mar 26, 2024

jhendersonHDF commented Mar 27, 2024

fortnern Mar 26, 2024 •

edited

Loading