heap-buffer-overflow in H5Odtype.c

[gabe-sherman]

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: 0394b03

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]

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
#1: ../../src/H5T.c line 3336 in H5T_decode(): can't decode object
major: Datatype
minor: Unable to decode value
#2: ../../src/H5Omessage.c line 1635 in H5O_msg_decode(): unable to decode message
major: Object header
minor: Unable to decode value
#3: ../../src/H5Oshared.h line 74 in H5O__dtype_shared_decode(): unable to decode native message
major: Object header
minor: Unable to decode value
#4: ../../src/H5Odtype.c line 1426 in H5O__dtype_decode(): can't decode type
major: Datatype
minor: Unable to decode value
#5: ../../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]

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]

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]

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?