Heap-buffer-overflow error found in void swapbytes(...) in src/databases/GMV/gmvread.c

[Arpan3323]

Describe the issue

Hi, I am reporting a bug found via mutation based fuzz-testing the function linked below.

visit/src/databases/GMV/gmvread.c

Line 4694 in 9b3fbd1

void swapbytes(void *from, int size, int nitems)

Simple test

A simple test that does not take into account how this function is used within the project and continuously throws random inputs at it, immediately results in a heap-buffer-overflow. Below are the details of this simple test.

Simple test code for reproducing the bug:

extern "C" int LLVMFuzzerTestOneInput(uint8_t *Data, size_t Size)
{
    if (Size < 2 || Size > 8) return 0;
    auto data = reinterpret_cast<void*>(Data);
    swapbytes(data, Size, 100);
    return 0;
}

Crash Report

INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 414403717
INFO: Loaded 1 modules   (129 inline 8-bit counters): 129 [0x561a9f355270, 0x561a9f3552f1), 
INFO: Loaded 1 PC tables (129 PCs): 129 [0x561a9f3552f8,0x561a9f355b08), 
INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes
INFO: A corpus is not provided, starting from an empty corpus
#2      INITED cov: 2 ft: 2 corp: 1/1b exec/s: 0 rss: 31Mb
=================================================================
==61311==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x5020000002b3 at pc 0x561a9f30e2f4 bp 0x7fffbb47c190 sp 0x7fffbb47c188
READ of size 1 at 0x5020000002b3 thread T0
    #0 0x561a9f30e2f3 in swapbytes(void*, int, int) build/_deps/visit-src/src/databases/GMV/gmvread.c:4744:20
    #1 0x561a9f30e386 in LLVMFuzzerTestOneInput /fuzz_gmvread/target_a/fuzzer.cpp:5:5
    #2 0x561a9f217c54 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/fuzz_gmvread/target_a/gmvReadFuzzer+0x4dc54) (BuildId: a65aa67be6e9e94a32910948b3b812093396451f)
    #3 0x561a9f217349 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) (/fuzz_gmvread/target_a/gmvReadFuzzer+0x4d349) (BuildId: a65aa67be6e9e94a32910948b3b812093396451f)
    #4 0x561a9f218b35 in fuzzer::Fuzzer::MutateAndTestOne() (/fuzz_gmvread/target_a/gmvReadFuzzer+0x4eb35) (BuildId: a65aa67be6e9e94a32910948b3b812093396451f)
    #5 0x561a9f219695 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/fuzz_gmvread/target_a/gmvReadFuzzer+0x4f695) (BuildId: a65aa67be6e9e94a32910948b3b812093396451f)
    #6 0x561a9f20696f in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/fuzz_gmvread/target_a/gmvReadFuzzer+0x3c96f) (BuildId: a65aa67be6e9e94a32910948b3b812093396451f)
    #7 0x561a9f230ff6 in main (/fuzz_gmvread/target_a/gmvReadFuzzer+0x66ff6) (BuildId: a65aa67be6e9e94a32910948b3b812093396451f)
    #8 0x7f7511d601c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 6d64b17fbac799e68da7ebd9985ddf9b5cb375e6)
    #9 0x7f7511d6028a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 6d64b17fbac799e68da7ebd9985ddf9b5cb375e6)
    #10 0x561a9f1fb954 in _start (/fuzz_gmvread/target_a/gmvReadFuzzer+0x31954) (BuildId: a65aa67be6e9e94a32910948b3b812093396451f)

0x5020000002b3 is located 1 bytes after 2-byte region [0x5020000002b0,0x5020000002b2)
allocated by thread T0 here:
    #0 0x561a9f30a4a1 in operator new[](unsigned long) (/fuzz_gmvread/target_a/gmvReadFuzzer+0x1404a1) (BuildId: a65aa67be6e9e94a32910948b3b812093396451f)
    #1 0x561a9f217b65 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/fuzz_gmvread/target_a/gmvReadFuzzer+0x4db65) (BuildId: a65aa67be6e9e94a32910948b3b812093396451f)
    #2 0x561a9f217349 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) (/fuzz_gmvread/target_a/gmvReadFuzzer+0x4d349) (BuildId: a65aa67be6e9e94a32910948b3b812093396451f)
    #3 0x561a9f218b35 in fuzzer::Fuzzer::MutateAndTestOne() (/fuzz_gmvread/target_a/gmvReadFuzzer+0x4eb35) (BuildId: a65aa67be6e9e94a32910948b3b812093396451f)
    #4 0x561a9f219695 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/fuzz_gmvread/target_a/gmvReadFuzzer+0x4f695) (BuildId: a65aa67be6e9e94a32910948b3b812093396451f)
    #5 0x561a9f20696f in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/fuzz_gmvread/target_a/gmvReadFuzzer+0x3c96f) (BuildId: a65aa67be6e9e94a32910948b3b812093396451f)
    #6 0x561a9f230ff6 in main (/fuzz_gmvread/target_a/gmvReadFuzzer+0x66ff6) (BuildId: a65aa67be6e9e94a32910948b3b812093396451f)
    #7 0x7f7511d601c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 6d64b17fbac799e68da7ebd9985ddf9b5cb375e6)
    #8 0x7f7511d6028a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 6d64b17fbac799e68da7ebd9985ddf9b5cb375e6)
    #9 0x561a9f1fb954 in _start (/fuzz_gmvread/target_a/gmvReadFuzzer+0x31954) (BuildId: a65aa67be6e9e94a32910948b3b812093396451f)

SUMMARY: AddressSanitizer: heap-buffer-overflow build/_deps/visit-src/src/databases/GMV/gmvread.c:4744:20 in swapbytes(void*, int, int)
Shadow bytes around the buggy address:
  0x502000000000: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fd fa
  0x502000000080: fa fa fd fa fa fa 00 fa fa fa 00 fa fa fa fd fa
  0x502000000100: fa fa 00 fa fa fa 01 fa fa fa 00 fa fa fa 00 00
  0x502000000180: fa fa 00 fa fa fa fd fa fa fa fd fa fa fa fd fd
  0x502000000200: fa fa fd fa fa fa fd fa fa fa 00 00 fa fa fd fa
=>0x502000000280: fa fa fd fa fa fa[02]fa fa fa fa fa fa fa fa fa
  0x502000000300: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x502000000380: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x502000000400: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x502000000480: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x502000000500: 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
==61311==ABORTING

Context-aware test

I was certain that while the simple test immediately broke the function, it might not be taking into account the context in which this function (void swapbytes(...)) is used within the project. I refined the test to supply inputs in a way that this function expects. This time the function did not break (based on ~1 hour of running the fuzzer binary).

Context-aware test code:

extern "C" int LLVMFuzzerTestOneInput(uint8_t *Data, size_t Size)
{
    if (Size < 2 || Size > 8) return 0;
    uint8_t *buffer = new uint8_t[Size];
    memcpy(buffer, Data, Size);
    swapbytes(buffer, sizeof(buffer), Size/sizeof(buffer));
    delete[] buffer;
    return 0;
}

Solution

The straightforward solution I can think of to avoid the heap-overflow triggered by the simple test would be to either take a global and architectural perspective at the problem and perhaps do some refactoring or update the function implementation. I do not have a precise solution this time but I still wanted to report the issue. Please feel free to close it, in case fixing this problem is not necessary.