DaveGamble/cJSON

OOB access in `parse_string`

oliverchang opened this issue · 2 comments

With the following fuzz target:

#include <stdint.h>
#include <stdlib.h>
#include <string.h>

#include "cJSON.h"

extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
  cJSON *json = cJSON_ParseWithLength((char *)data, size);
  if (json) {
    cJSON_Delete(json);
  }
  return 0;
}

And this input (with no trailing newline)

{"1":1,

We get the following ASan OOB read report:

$  echo -ne '{"1":1,' > input
$ ./fuzzer input

=================================================================
==102538==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000000037 at pc 0x55a3fbc9fae6 bp 0x7fffbfab0d20 sp 0x7fffbfab0d18
READ of size 1 at 0x602000000037 thread T0
    #0 0x55a3fbc9fae5 in parse_string cJSON.c
    #1 0x55a3fbca4049 in parse_object cJSON.c
    #2 0x55a3fbca2d27 in parse_value cJSON.c
    #3 0x55a3fbca141a in cJSON_ParseWithLengthOpts (/usr/local/google/home/ochang/cJSON/fuzzer2+0x12241a) (BuildId: daf1c240e8c9734954177325768577e23be337d5)
    #4 0x55a3fbca1782 in cJSON_ParseWithLength (/usr/local/google/home/ochang/cJSON/fuzzer2+0x122782) (BuildId: daf1c240e8c9734954177325768577e23be337d5)
    #5 0x55a3fbc9d444 in LLVMFuzzerTestOneInput (/usr/local/google/home/ochang/cJSON/fuzzer2+0x11e444) (BuildId: daf1c240e8c9734954177325768577e23be337d5)
    #6 0x55a3fbbc3413 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/usr/local/google/home/ochang/cJSON/fuzzer2+0x44413) (BuildId: daf1c240e8c9734954177325768577e23be337d5)
    #7 0x55a3fbbac49f in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (/usr/local/google/home/ochang/cJSON/fuzzer2+0x2d49f) (BuildId: daf1c240e8c9734954177325768577e23be337d5)
    #8 0x55a3fbbb2326 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/usr/local/google/home/ochang/cJSON/fuzzer2+0x33326) (BuildId: daf1c240e8c9734954177325768577e23be337d5)
    #9 0x55a3fbbdd2b2 in main (/usr/local/google/home/ochang/cJSON/fuzzer2+0x5e2b2) (BuildId: daf1c240e8c9734954177325768577e23be337d5)
    #10 0x7fd1eba456c9 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #11 0x7fd1eba45784 in __libc_start_main csu/../csu/libc-start.c:360:3
    #12 0x55a3fbba6940 in _start (/usr/local/google/home/ochang/cJSON/fuzzer2+0x27940) (BuildId: daf1c240e8c9734954177325768577e23be337d5)

0x602000000037 is located 0 bytes to the right of 7-byte region [0x602000000030,0x602000000037)
allocated by thread T0 here:
    #0 0x55a3fbc9aefd in operator new[](unsigned long) (/usr/local/google/home/ochang/cJSON/fuzzer2+0x11befd) (BuildId: daf1c240e8c9734954177325768577e23be337d5)
    #1 0x55a3fbbc3322 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/usr/local/google/home/ochang/cJSON/fuzzer2+0x44322) (BuildId: daf1c240e8c9734954177325768577e23be337d5)
    #2 0x55a3fbbac49f in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (/usr/local/google/home/ochang/cJSON/fuzzer2+0x2d49f) (BuildId: daf1c240e8c9734954177325768577e23be337d5)
    #3 0x55a3fbbb2326 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/usr/local/google/home/ochang/cJSON/fuzzer2+0x33326) (BuildId: daf1c240e8c9734954177325768577e23be337d5)
    #4 0x55a3fbbdd2b2 in main (/usr/local/google/home/ochang/cJSON/fuzzer2+0x5e2b2) (BuildId: daf1c240e8c9734954177325768577e23be337d5)
    #5 0x7fd1eba456c9 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16

SUMMARY: AddressSanitizer: heap-buffer-overflow cJSON.c in parse_string
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 07 fa fa fa[07]fa fa fa 03 fa fa fa fa fa
  0x0c047fff8010: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8020: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8050: 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
==102538==ABORTING

Note this isn't caught by the existing fuzz target because it enforces the input having a null terminator, which hides this OOB access:

if(data[size-1] != '\0') return 0;

Gentle ping on this issue!

I've submitted a possible fix. Though only for the specific bug caught, and not the issue with the existing fuzz target being unable to catch such a bug. That's also important to fix.