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

A memory leak was found while using H5F__accum_write #4585

Closed
MageWeiG opened this issue Jun 19, 2024 · 2 comments · Fixed by #4882
Closed

A memory leak was found while using H5F__accum_write #4585

MageWeiG opened this issue Jun 19, 2024 · 2 comments · Fixed by #4882
Assignees
Labels
Component - C Library Core C library issues (usually in the src directory) Priority - 0. Blocker ⛔ This MUST be merged for the release to happen Type - Bug Please report security issues to help@hdfgroup.org instead of creating an issue on GitHub Type - Security Security issues, including library crashers and memory leaks
Milestone

Comments

@MageWeiG
Copy link

When I was testing h5_read_fuzzer with libfuzzer, I found a memory leak .

The cause of this vulnerability is: H5F__accum_write calls H5FL_blk_malloc to apply for memory, and after a series of operations such as H5F_block_write and H5F__flush_phase2 functions do not release successfully, resulting in the vulnerability.

with the following crash information:

=================================================================
==13==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 72 byte(s) in 1 object(s) allocated from:
    #0 0x55e010de9c5e in malloc /src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:69:3
    #1 0x55e010fbfd96 in H5FL__malloc /src/hdf5/src/H5FL.c:203:30
    #2 0x55e010fbfd96 in H5FL_blk_malloc /src/hdf5/src/H5FL.c:765:48
    #3 0x55e0116a514e in H5F__accum_write /src/hdf5/src/H5Faccum.c:700:47
    #4 0x55e0111a80a2 in H5PB_write /src/hdf5/src/H5PB.c:1000:13
    #5 0x55e010f78cf6 in H5F_block_write /src/hdf5/src/H5Fio.c:218:9
    #6 0x55e010e8321f in H5C__flush_single_entry /src/hdf5/src/H5Centry.c:597:21
    #7 0x55e010eadcf1 in H5C__flush_ring /src/hdf5/src/H5Cint.c:1748:25
    #8 0x55e010e7b4a7 in H5C_flush_cache /src/hdf5/src/H5C.c:707:17
    #9 0x55e010e3e0ff in H5AC_flush /src/hdf5/src/H5AC.c:619:9
    #10 0x55e010f6fced in H5F__flush_phase2 /src/hdf5/src/H5Fint.c:2305:9
    #11 0x55e010f6dab4 in H5F__dest /src/hdf5/src/H5Fint.c:1404:17
    #12 0x55e010f71074 in H5F_try_close /src/hdf5/src/H5Fint.c:2644:9
    #13 0x55e010f70585 in H5F__close /src/hdf5/src/H5Fint.c:2446:9
    #14 0x55e011589d91 in H5VL__native_file_close /src/hdf5/src/H5VLnative_file.c:782:13
    #15 0x55e01155d58c in H5VL__file_close /src/hdf5/src/H5VLcallback.c:4130:9
    #16 0x55e01155d58c in H5VL_file_close /src/hdf5/src/H5VLcallback.c:4161:9
    #17 0x55e010f77649 in H5F__close_cb /src/hdf5/src/H5Fint.c:217:9
    #18 0x55e01107a07c in H5I__dec_ref /src/hdf5/src/H5Iint.c:973:43
    #19 0x55e01107a27d in H5I__dec_app_ref /src/hdf5/src/H5Iint.c:1044:22
    #20 0x55e01107a27d in H5I_dec_app_ref /src/hdf5/src/H5Iint.c:1089:22
    #21 0x55e010f4eda6 in H5Fclose /src/hdf5/src/H5F.c:1046:9
    #22 0x55e010e28e7d in LLVMFuzzerTestOneInput /src/h5_read_fuzzer.c:40:9
    #23 0x55e010cdb6c0 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:614:13
    #24 0x55e010cdaee5 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:516:7
    #25 0x55e010cdcd36 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__Fuzzer::vector<fuzzer::SizedFile, std::__Fuzzer::allocator<fuzzer::SizedFile>>&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:812:5
    #26 0x55e010cdd1a7 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, std::__Fuzzer::allocator<fuzzer::SizedFile>>&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:867:3
    #27 0x55e010ccb7b6 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:914:6
    #28 0x55e010cf7ce2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
    #29 0x7f6c7f765082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 0702430aef5fa3dda43986563e9ffcc47efbd75e)

DEDUP_TOKEN: __interceptor_malloc--H5FL__malloc--H5FL_blk_malloc
SUMMARY: AddressSanitizer: 72 byte(s) leaked in 1 allocation(s).
INFO: to ignore leaks on libFuzzer side use -detect_leaks=0.

MS: 2 ChangeByte-ShuffleBytes-; base unit: c7cf6e422300e8b9931f04c1f45c11e4cd20ae42
0x25,0x89,0x48,0x44,0x46,0xd,0xa,0x1a,0xa,0x0,0x0,0x0,0x14,0x0,0x2,0x2,0x30,0x44,0xb4,0xd,0x0,0x0,0x0,0x0,0x0,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0x0,0xff,0x0,0x0,0x0,0x0,
%\211HDF\015\012\032\012\000\000\000\024\000\002\0020D\264\015\000\000\000\000\000\377\377\377\377\377\377\377\000\377\000\000\000\000
artifact_prefix='./'; Test unit written to ./leak-6885bf9dd873fb75b3ef6bb2c03afbb2b0f0d6af
Base64: JYlIREYNChoKAAAAFAACAjBEtA0AAAAAAP////////8A/wAAAAA=

The vulnerability trigger sample is attached.
leak-6885bf9dd873fb75b3ef6bb2c03afbb2b0f0d6af.zip

@derobins derobins added this to the 1.14.5 milestone Jun 19, 2024
@derobins derobins added Priority - 0. Blocker ⛔ This MUST be merged for the release to happen Component - C Library Core C library issues (usually in the src directory) Type - Bug Please report security issues to help@hdfgroup.org instead of creating an issue on GitHub labels Jun 19, 2024
@derobins
Copy link
Member

Which version of the library is this, how are you configuring and building the library, and how are you exercising the code? I don't see this problem via valgrind w/ gcc 13.2.0 when I run the file through the HDF5 command-line tools. It's looking for a custom VFD, though and the errors I'm seeing are plugin search errors, not file parse errors, which is where we typically have problems with fuzzers.

@MageWeiG
Copy link
Author

This vulnerability was discovered while testing h5_read_fuzzer using ossfuzz. According to Dockerfile (https://github.com/google/oss-fuzz/blob/master/projects/hdf5/Dockerfile), using the latest code.
Because in the build. Sh (https://github.com/google/oss-fuzz/blob/master/projects/hdf5/build.sh) file is not found in the compiler, so I run into the Docker environment to see, The default compilation environment is 'CXX=clang++','CC=clang'.
The harness file used is located https://github.com/google/oss-fuzz/blob/master/projects/hdf5/h5_read_fuzzer.c .
Just replicated it using fuzzer and found it triggers. Do you need any more information?

@derobins derobins added Type - Security Security issues, including library crashers and memory leaks labels Jun 25, 2024
@bmribler bmribler assigned bmribler and unassigned derobins Sep 23, 2024
bmribler added a commit to bmribler/hdf5_bmr23 that referenced this issue Sep 27, 2024
bmribler added a commit to bmribler/hdf5_bmr23 that referenced this issue Sep 27, 2024
lrknox pushed a commit that referenced this issue Sep 27, 2024
lrknox pushed a commit to lrknox/hdf5 that referenced this issue Sep 27, 2024
lrknox added a commit that referenced this issue Sep 28, 2024
* Fix issues with large external data files (#4843) (#4847)

* Fixed a memory leak from H5FL_blk_malloc (#4882)

In H5F__accum_reset(), when H5F__accum_flush() failed, the freeing of
f_sh->accum.buf was never reached, causing resource leak.

@fortnern added the third argument to H5F__accum_reset() so we can free
f_sh->accum.buf when we close the file, that is, when H5F__accum_reset()
is called from the H5F__dest() route, and can leave the accumulator in place
otherwise.

* Added an entry for the GH-4585 fix (#4889)

* Fix an incorrect returned value by H5LTfind_dataset() (#4869)

H5LTfind_dataset() returns true for non-existing datasets because it only compares up to the length of the searched string, such as "Day" vs "DayNight" (issue GH-4780).

This PR applied the user's patch and added tests.

* Fix minor spelling in documentation (#4870)

---------

Co-authored-by: Neil Fortner <fortnern@gmail.com>
Co-authored-by: bmribler <39579120+bmribler@users.noreply.github.com>
lrknox added a commit that referenced this issue Sep 28, 2024
* Fix issues with large external data files (#4843) (#4847)

* Fixed a memory leak from H5FL_blk_malloc (#4882)

In H5F__accum_reset(), when H5F__accum_flush() failed, the freeing of
f_sh->accum.buf was never reached, causing resource leak.

@fortnern added the third argument to H5F__accum_reset() so we can free
f_sh->accum.buf when we close the file, that is, when H5F__accum_reset()
is called from the H5F__dest() route, and can leave the accumulator in place
otherwise.

* Added an entry for the GH-4585 fix (#4889)

* Fix an incorrect returned value by H5LTfind_dataset() (#4869)

H5LTfind_dataset() returns true for non-existing datasets because it only compares up to the length of the searched string, such as "Day" vs "DayNight" (issue GH-4780).

This PR applied the user's patch and added tests.

* Fix minor spelling in documentation (#4870)

* Updated Platforms tested in RELEASE.txt
Incremented version subrelease to -3.
lrknox added a commit that referenced this issue Sep 30, 2024
* Fix issues with large external data files (#4843) (#4847)

* Fixed a memory leak from H5FL_blk_malloc (#4882)

In H5F__accum_reset(), when H5F__accum_flush() failed, the freeing of
f_sh->accum.buf was never reached, causing resource leak.

@fortnern added the third argument to H5F__accum_reset() so we can free
f_sh->accum.buf when we close the file, that is, when H5F__accum_reset()
is called from the H5F__dest() route, and can leave the accumulator in place
otherwise.

* Added an entry for the GH-4585 fix (#4889)

* Fix an incorrect returned value by H5LTfind_dataset() (#4869)

H5LTfind_dataset() returns true for non-existing datasets because it only compares up to the length of the searched string, such as "Day" vs "DayNight" (issue GH-4780).

This PR applied the user's patch and added tests.

* Fix minor spelling in documentation (#4870)

* Set release version 1.14.5 and release date to 2024-09-30.
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 - 0. Blocker ⛔ This MUST be merged for the release to happen Type - Bug Please report security issues to help@hdfgroup.org instead of creating an issue on GitHub Type - Security Security issues, including library crashers and memory leaks
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants