GPMF-parser: Heap-buffer-overflow in GPMF_FormattedData due to integer truncation in size validation

[xnqjns]

Vulnerability Summary

A heap overflow vulnerability exists in the GPMF_FormattedData function. This vulnerability stems from an overly coarse validation logic for the remaining size of the buffer (using a right shift >> 2), which allows the validation to be bypassed and an out-of-bounds read (OOB Read) to be triggered when the remaining bytes are not a multiple of 4.

GPMF_ERR GPMF_FormattedData(GPMF_stream *ms, void *buffer, uint32_t buffersize, uint32_t sample_offset, uint32_t read_samples)
{
	if (ms && buffer)
	{
		uint8_t *data = (uint8_t *)&ms->buffer[ms->pos + 2];
		uint8_t *output = (uint8_t *)buffer;
		uint32_t sample_size = GPMF_SAMPLE_SIZE(ms->buffer[ms->pos + 1]);
		uint32_t remaining_sample_size = GPMF_DATA_PACKEDSIZE(ms->buffer[ms->pos + 1]);
		uint8_t type = GPMF_SAMPLE_TYPE(ms->buffer[ms->pos + 1]);
		uint32_t typesize = 1;
		uint32_t elements = 0;
		uint32_t typestringlength = 1;
		char complextype[64] = "L";

		if (type == GPMF_TYPE_NEST)
			return GPMF_ERROR_BAD_STRUCTURE;
		
		if (GPMF_OK != IsValidSize(ms, remaining_sample_size>>2)) //Affected code
			return GPMF_ERROR_BAD_STRUCTURE;
 

The use of bitwise right-shift (>> 2) performs a floor division by 4. If the buffer has remaining bytes that are not a multiple of 4 (e.g., 5 or 6 bytes), the check may pass even if a subsequent 2-byte or 4-byte read exceeds the actual physical boundary. In this specific crash, the logic enters case 2

case 2:
{
    uint16_t *data16 = (uint16_t *)data;
    *output16 = BYTESWAP16(*data16); // OOB Read occurs here
}

This issue also occurs in the default branch, but I've lost the corresponding crash file, but they both involve the same vulnerability.

default: //1, 16 or more not byteswapped
					for (j = 0; j < typesize; j++)
						*output++ = *data++; // OOB Read occurs here
					break;

==547763==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x5120000005dc at pc 0x559d4f9804d6 bp 0x7ffdcc69b6f0 sp 0x7ffdcc69b6e8
READ of size 2 at 0x5120000005dc thread T0
    #0 0x559d4f9804d5 in GPMF_FormattedData /workspace/gpmf-parser/GPMF_parser.c:1262:18
    #1 0x559d4f96f915 in TestOneProtoInput(GPMFfuzzer::GPMF_Node const&) /workspace/gpmf-parser/harness.cc:130:21
    #2 0x559d4f96f915 in LLVMFuzzerTestOneInput /workspace/gpmf-parser/harness.cc:91:1
    #3 0x559d4f86fd9a in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/workspace/gpmf-parser/gpmf_libfuzzer+0x6fd9a) (BuildId: e737819d00e3fa2d493ce01c8a162b141ec22731)
    #4 0x559d4f857a33 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (/workspace/gpmf-parser/gpmf_libfuzzer+0x57a33) (BuildId: e737819d00e3fa2d493ce01c8a162b141ec22731)
    #5 0x559d4f85dbf1 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/workspace/gpmf-parser/gpmf_libfuzzer+0x5dbf1) (BuildId: e737819d00e3fa2d493ce01c8a162b141ec22731)
    #6 0x559d4f88a1c6 in main (/workspace/gpmf-parser/gpmf_libfuzzer+0x8a1c6) (BuildId: e737819d00e3fa2d493ce01c8a162b141ec22731)
    #7 0x7f29b402a1c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
    #8 0x7f29b402a28a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
    #9 0x559d4f8521c4 in _start (/workspace/gpmf-parser/gpmf_libfuzzer+0x521c4) (BuildId: e737819d00e3fa2d493ce01c8a162b141ec22731)

0x5120000005dc is located 0 bytes after 284-byte region [0x5120000004c0,0x5120000005dc)
allocated by thread T0 here:
    #0 0x559d4f96b591 in operator new(unsigned long) (/workspace/gpmf-parser/gpmf_libfuzzer+0x16b591) (BuildId: e737819d00e3fa2d493ce01c8a162b141ec22731)
    #1 0x559d4f9702df in std::__new_allocator<unsigned char>::allocate(unsigned long, void const*) /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/new_allocator.h:151:27
    #2 0x559d4f9702df in std::allocator_traits<std::allocator<unsigned char>>::allocate(std::allocator<unsigned char>&, unsigned long) /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/alloc_traits.h:482:20
    #3 0x559d4f9702df in std::_Vector_base<unsigned char, std::allocator<unsigned char>>::_M_allocate(unsigned long) /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_vector.h:381:20
    #4 0x559d4f9702df in std::_Vector_base<unsigned char, std::allocator<unsigned char>>::_M_create_storage(unsigned long) /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_vector.h:398:33
    #5 0x559d4f9702df in std::_Vector_base<unsigned char, std::allocator<unsigned char>>::_Vector_base(unsigned long, std::allocator<unsigned char> const&) /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_vector.h:335:9
    #6 0x559d4f9702df in std::vector<unsigned char, std::allocator<unsigned char>>::vector(unsigned long, unsigned char const&, std::allocator<unsigned char> const&) /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_vector.h:571:9
    #7 0x559d4f96f6d6 in TestOneProtoInput(GPMFfuzzer::GPMF_Node const&) /workspace/gpmf-parser/harness.cc:101:26
    #8 0x559d4f96f6d6 in LLVMFuzzerTestOneInput /workspace/gpmf-parser/harness.cc:91:1
    #9 0x559d4f86fd9a in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/workspace/gpmf-parser/gpmf_libfuzzer+0x6fd9a) (BuildId: e737819d00e3fa2d493ce01c8a162b141ec22731)
    #10 0x559d4f857a33 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (/workspace/gpmf-parser/gpmf_libfuzzer+0x57a33) (BuildId: e737819d00e3fa2d493ce01c8a162b141ec22731)
    #11 0x559d4f85dbf1 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/workspace/gpmf-parser/gpmf_libfuzzer+0x5dbf1) (BuildId: e737819d00e3fa2d493ce01c8a162b141ec22731)
    #12 0x559d4f88a1c6 in main (/workspace/gpmf-parser/gpmf_libfuzzer+0x8a1c6) (BuildId: e737819d00e3fa2d493ce01c8a162b141ec22731)
    #13 0x7f29b402a1c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
    #14 0x7f29b402a28a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
    #15 0x559d4f8521c4 in _start (/workspace/gpmf-parser/gpmf_libfuzzer+0x521c4) (BuildId: e737819d00e3fa2d493ce01c8a162b141ec22731)

SUMMARY: AddressSanitizer: heap-buffer-overflow /workspace/gpmf-parser/GPMF_parser.c:1262:18 in GPMF_FormattedData
Shadow bytes around the buggy address:
  0x512000000300: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x512000000380: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x512000000400: 00 00 00 00 00 00 00 00 00 00 00 05 fa fa fa fa
  0x512000000480: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x512000000500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x512000000580: 00 00 00 00 00 00 00 00 00 00 00[04]fa fa fa fa
  0x512000000600: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x512000000680: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x512000000700: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x512000000780: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x512000000800: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
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
==547763==ABORTING

[xnqjns]

I originally planned to fix it myself and then continue with the fuzz test. I tried a few modifications.

1. uint32_t required_words = (uint32_t)((((uint64_t)remaining_sample_size + 3) / 4) + 2);
   I changed it to round up (+3 then /4) to ensure that no byte is missed, and also added +2 to compensate for the physical offset of the header.
2. uint64_t skip_bytes = (uint64_t)sample_offset * sample_size; uint64_t read_bytes = (uint64_t)sample_size * read_samples;
   I forced all critical multiplication calculations to 64 bits, which should prevent all overflows.
3. When processing COMPRESSED (compressed stream), the code placed the decompressed data into the output buffer and overwrote the original type variable. This caused subsequent code to completely "forget" that it was processing decompressed data, incorrectly using the physical length of the original file to measure the read boundary of the output buffer, ultimately resulting in an out-of-bounds error.
   I added an is_compressed label to force the program to remember the current state. If the data points to internal output, it must be governed by buffersize.

However, even after these three modifications, the corresponding crash file is still being triggered. I don't know where the problem lies.

[dnewman-gpsw]

Can you create a test file that creates this vulnerability? I understand it, but need to a speedy way to verify and test.

[xnqjns]

crash-f45aec1b5c7a0e7d13d48c1997a11026bebe5ff4.zip
I have attached a .zip archive containing the raw crash file generated by the fuzzer. You can feed this binary payload directly into the parser to trigger the ASan heap-buffer-overflow.
Let me know if you need any additional debugging information or testing on my end once you have a patch!