HDFGroup / hdf5

Official HDF5® Library Repository
https://www.hdfgroup.org/
Other
626 stars 256 forks source link

heap-buffer-overflow in H5Odtype.c #4436

Open gabe-sherman opened 7 months ago

gabe-sherman commented 7 months ago

A heap-buffer-overflow occurs in the below program. This behavior occurs at line 149 in H5Odtype.c.

#include "hdf5.h"
#include <stdbool.h>
#include <stdint.h>
#include <stdlib.h>
#include <string.h>
#include <stdio.h>
#include <math.h>
typedef uint8_t   u8;   
typedef uint16_t  u16;  
typedef uint32_t  u32;  
typedef uint64_t  u64;
typedef unsigned int usize;
typedef int8_t  i8;
typedef int16_t i16;
typedef int32_t i32;
typedef int64_t i64;
typedef int isize;
typedef float f32;
typedef double f64;
int main() {
    i8 v0_tmp[] = {3, 0, }; // buf
    i8 *v0 = malloc(sizeof v0_tmp);
    memcpy(v0, v0_tmp, sizeof v0_tmp);
    i8 *v1 = v0; // buf
    i64 v2 = H5Tdecode(v1); // $target
}

How to trigger

./filename

Test Environment

Ubuntu 22.04, 64bit

Version

Latest: 0394b03f66dc45fe96e2c772b7bce293e4316ad2

Address Sanitizer Output

=================================================================
==1448886==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000000012 at pc 0x5555576cd240 bp 0x7fffffffd4b0 sp 0x7fffffffd4a8
READ of size 1 at 0x602000000012 thread T0
    #0 0x5555576cd23f in H5O__dtype_decode_helper /home/gabesherman/harness_test/AutoHarn-Evaluation/hdf5/lib_asan/src/H5Odtype.c:149:5
    #1 0x5555576b069a in H5O__dtype_decode /home/gabesherman/harness_test/AutoHarn-Evaluation/hdf5/lib_asan/src/H5Odtype.c:1425:9
    #2 0x5555576b069a in H5O__dtype_shared_decode /home/gabesherman/harness_test/AutoHarn-Evaluation/hdf5/lib_asan/src/./H5Oshared.h:73:34
    #3 0x55555777fe7e in H5O_msg_decode /home/gabesherman/harness_test/AutoHarn-Evaluation/hdf5/lib_asan/src/H5Omessage.c:1634:30
    #4 0x555556656571 in H5T_decode /home/gabesherman/harness_test/AutoHarn-Evaluation/hdf5/lib_asan/src/H5T.c:3326:39
    #5 0x555556655d3f in H5Tdecode /home/gabesherman/harness_test/AutoHarn-Evaluation/hdf5/lib_asan/src/H5T.c:3225:23
    #6 0x5555565f32ed in main /home/gabesherman/harness_test/AutoHarn-Results/hdf5/hopper-04/reproducer.c:25:14
    #7 0x7ffff7c29d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #8 0x7ffff7c29e3f in __libc_start_main csu/../csu/libc-start.c:392:3
    #9 0x555556535614 in _start (/home/gabesherman/harness_test/AutoHarn-Results/hdf5/hopper-04/reproducer+0xfe1614) (BuildId: 8b9501cb5c2971555d44192cef0459eadbf27672)

0x602000000012 is located 0 bytes to the right of 2-byte region [0x602000000010,0x602000000012)
allocated by thread T0 here:
    #0 0x5555565b845e in __interceptor_malloc (/home/gabesherman/harness_test/AutoHarn-Results/hdf5/hopper-04/reproducer+0x106445e) (BuildId: 8b9501cb5c2971555d44192cef0459eadbf27672)
    #1 0x5555565f328e in main /home/gabesherman/harness_test/AutoHarn-Results/hdf5/hopper-04/reproducer.c:22:14
    #2 0x7ffff7c29d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/gabesherman/harness_test/AutoHarn-Evaluation/hdf5/lib_asan/src/H5Odtype.c:149:5 in H5O__dtype_decode_helper
Shadow bytes around the buggy address:
  0x0c047fff7fb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c047fff8000: fa fa[02]fa fa fa 00 00 fa fa 05 fa fa fa 05 fa
  0x0c047fff8010: fa fa 07 fa fa fa 00 00 fa fa 00 00 fa fa 00 05
  0x0c047fff8020: fa fa 00 02 fa fa 00 05 fa fa 00 03 fa fa 00 04
  0x0c047fff8030: fa fa 00 01 fa fa 00 07 fa fa 06 fa fa fa 00 05
  0x0c047fff8040: fa fa 00 02 fa fa 00 02 fa fa 00 03 fa fa 00 04
  0x0c047fff8050: fa fa 00 07 fa fa 00 fa fa fa 00 02 fa fa 00 02
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==1448886==ABORTING
bmribler commented 5 months ago

I did not see the heap-buffer-overflow, only a small memory leaks:

HDF5-DIAG: Error detected in HDF5 (1.15.0):

000: ../../src/H5T.c line 3235 in H5Tdecode(): can't decode object

major: Datatype
minor: Unable to decode value

001: ../../src/H5T.c line 3336 in H5T_decode(): can't decode object

major: Datatype
minor: Unable to decode value

002: ../../src/H5Omessage.c line 1635 in H5O_msg_decode(): unable to decode message

major: Object header
minor: Unable to decode value

003: ../../src/H5Oshared.h line 74 in H5O__dtype_shared_decode(): unable to decode native message

major: Object header
minor: Unable to decode value

004: ../../src/H5Odtype.c line 1426 in H5O__dtype_decode(): can't decode type

major: Datatype
minor: Unable to decode value

005: ../../src/H5Odtype.c line 152 in H5O__dtype_decode_helper(): bad version number for datatype message

major: Datatype
minor: Unable to load metadata into cache

================================================================= ==492430==ERROR: LeakSanitizer: detected memory leaks

After I added free(v0), the memory leaks were gone too. Please advise.

mattjala commented 5 months ago

H5Tdecode() doesn't take a parameter specifying the size of the buffer, so it's not possible for the library to do normal bounds checking on it - instead, bounds checking has to be skipped (see H5_IS_KNOWN_BUFFER_OVERFLOW).

I'm not sure there is a solution for 'user passes a malformed buffer to H5Tdecode()' short of deprecating it in favor of a new H5Tdecode2() which take the parameters necessary to do real bounds checking.

derobins commented 4 months ago

H5Tdecode() doesn't take a parameter specifying the size of the buffer, so it's not possible for the library to do normal bounds checking on it - instead, bounds checking has to be skipped (see H5_IS_KNOWN_BUFFER_OVERFLOW).

I'm not sure there is a solution for 'user passes a malformed buffer to H5Tdecode()' short of deprecating it in favor of a new H5Tdecode2() which take the parameters necessary to do real bounds checking.

We should create an issue for 2.0.0 to update the API call to take a buffer size.

bmribler commented 1 month ago

H5Tdecode() doesn't take a parameter specifying the size of the buffer, so it's not possible for the library to do normal bounds checking on it - instead, bounds checking has to be skipped (see H5_IS_KNOWN_BUFFER_OVERFLOW). I'm not sure there is a solution for 'user passes a malformed buffer to H5Tdecode()' short of deprecating it in favor of a new H5Tdecode2() which take the parameters necessary to do real bounds checking.

We should create an issue for 2.0.0 to update the API call to take a buffer size.

If H5Tdecode() is going to be deprecated, should this issue be closed?