Invalid out of bounds stack memory read on some inputs in function iniparser_load()

[hannob]

An out of bounds stack read access can happen in the function iniparser_load(). This can be triggered by parsing a file that simply contains a zero-byte (create with e.g. "dd if=/dev/zero of=zerobytefile bs=1 count=1"). To see this you can use address sanitizer (add -fsanitize=address in CFLAGS).

This is the code causing this:

    while (fgets(line+last, ASCIILINESZ-last, in)!=NULL) {
        lineno++ ;
        len = (int)strlen(line)-1;
        if (len==0)
            continue;
        /* Safety check against buffer overflows */
        if (line[len]!='\n' && !feof(in)) {

The problem here is that when the line consists of a zero byte then strlen(line) is 0 and len becomes -1. Therefore line[-1] is read and that is invalid memory.

This could be fixed by changing the check for len to <=0, but I'm not sure this is a good solution (this only works because len is signed, it probably should be unsigned anyway).

This issue was found with the tool american fuzzy lop. Here's the error message from address sanitizer:

==1691==ERROR: AddressSanitizer: stack-buffer-underflow on address 0x7ffceaee49ff at pc 0x0000004e5486 bp 0x7ffceaee49d0 sp 0x7ffceaee49c8
READ of size 1 at 0x7ffceaee49ff thread T0
    #0 0x4e5485 in iniparser_load /f/iniparser/iniparser/src/iniparser.c:684:13
    #1 0x4df09a in main /f/iniparser/iniparser/example/parse.c:19:11
    #2 0x7f938881a7af in __libc_start_main (/lib64/libc.so.6+0x207af)
    #3 0x417e38 in _start (/mnt/ram/iniparser/parse+0x417e38)

Address 0x7ffceaee49ff is located in stack of thread T0 at offset 31 in frame
    #0 0x4e318f in iniparser_load /f/iniparser/iniparser/src/iniparser.c:645

  This frame has 5 object(s):
    [32, 1057) 'line' <== Memory access at offset 31 underflows this variable
    [1200, 2225) 'section'
    [2368, 3393) 'key'
    [3536, 5585) 'tmp'
    [5728, 6753) 'val'
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-underflow /f/iniparser/iniparser/src/iniparser.c:684:13 in iniparser_load
Shadow bytes around the buggy address:
  0x10001d5d48e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10001d5d48f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10001d5d4900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10001d5d4910: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10001d5d4920: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x10001d5d4930: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1[f1]
  0x10001d5d4940: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10001d5d4950: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10001d5d4960: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10001d5d4970: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10001d5d4980: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
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
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  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
==1691==ABORTING

[touilleMan]

Sorry for the delay of the answer

I think the len<=0 is acceptable given len is an int and strlen is already casted to int

[hartwork]

It would be interesting if versions 2.17 and 3.1 are affected. Any ideas?

[hannob]

@hartwork 3.1 is affected, 2.17 is not.
But according to the webpage 2.17 is "For archeological purposes only" so I guess it's not recommended to use it.

[hartwork]

Okay thanks! For 2.17 I was asking because I have seen it bundled. I agree it should not be used in 2018.