stack buffer overflows during exception handling

[hongxuchen]

We found with our fuzzer some stack buffer overflows during error handling with address sanitizer.

=================================================================
==31892==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffc56758c74 at pc 0x55f1de47add9 bp 0x7ffc567588a0 sp 0x7ffc56758890
READ of size 1 at 0x7ffc56758c74 thread T0
    #0 0x55f1de47add8 in jsvStringIteratorPrintfCallback src/jsvar.c:1567
    #1 0x55f1de48ec88 in vcbprintf src/jsutils.c:739
    #2 0x55f1de48d010 in jsExceptionHere src/jsutils.c:240
    #3 0x55f1de46bb44 in jslMatch src/jslex.c:869
    #4 0x55f1de4a19f0 in jspeStatement src/jsparse.c:2730
    #5 0x55f1de49d0a1 in jspeBlockOrStatement src/jsparse.c:2079
    #6 0x55f1de49d1ab in jspParse src/jsparse.c:2091
    #7 0x55f1de4a240c in jspEvaluateVar src/jsparse.c:2899
    #8 0x55f1de4a2767 in jspEvaluate src/jsparse.c:2931
    #9 0x55f1de549c8d in main targets/linux/main.c:330
    #10 0x7f9dae057b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
    #11 0x55f1de465119 in _start (/home/hongxu/tests/Espruino-asan/espruino+0x36119)

Address 0x7ffc56758c74 is located in stack of thread T0 at offset 52 in frame
    #0 0x55f1de46b996 in jslMatch src/jslex.c:860

  This frame has 2 object(s):
    [32, 52) 'gotStr' <== Memory access at offset 52 overflows this variable
    [96, 116) 'expStr'
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-overflow src/jsvar.c:1567 in jsvStringIteratorPrintfCallback
Shadow bytes around the buggy address:
  0x10000ace3130: 00 f2 f2 f2 f2 f2 f2 f2 00 00 00 00 f3 f3 f3 f3
  0x10000ace3140: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10000ace3150: f1 f1 f1 f1 00 00 00 00 00 f2 f2 f2 f2 f2 f2 f2
  0x10000ace3160: 00 00 00 f2 00 00 00 00 00 00 00 00 00 00 00 00
  0x10000ace3170: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x10000ace3180: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00[04]f2
  0x10000ace3190: f2 f2 f2 f2 00 00 04 f2 00 00 00 00 00 00 00 00
  0x10000ace31a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10000ace31b0: 00 00 00 00 f1 f1 f1 f1 00 00 00 00 00 00 00 00
  0x10000ace31c0: 00 00 00 00 00 f2 f2 f2 f2 f2 f2 f2 00 00 00 00
  0x10000ace31d0: 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
  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
==31892==ABORTING

sampe input files:
eh_0.txt
eh_1.txt

[gfwilliams]

Again, all I see is:

./espruino ~/Downloads/eh_0.txt 

 ____                 _ 
|  __|___ ___ ___ _ _|_|___ ___ 
|  __|_ -| . |  _| | | |   | . |
|____|___|  _|_| |___|_|_|_|___|
         |_| espruino.com
 1v98 (c) 2018 G.Williams

Espruino is Open Source. Our work is supported
only by sales of official boards and donations:
http://espruino.com/Donate

Uncaught SyntaxError: Got UNFINISHED TEMPLATE  expected '{'

Now the error should read "UNFINISHED TEMPLATE LITERAL", and that's because jslMatch's gotStr buffer is too small (which is a bug which i'll fix) - but I use strncpy to copy the string in, so it should be properly bounded and zero-terminated, so wouldn't cause a problem - and doesn't appear to on my computer.

Please could you explain why you think AddressSanitizer has flagged an error, and what could be modified in the code to fix it? It seems to me that this and some of the other stuff you flagged is related to gotStr, which seems fine.

[hongxuchen]

I think this is relevant to the use of strncpy:

If there is no terminating null byte in the first n bytes of src, strncpy() produces an unterminated string in dest. If buf has length buflen, you can force termination using
something like the following:

        strncpy(buf, str, buflen - 1);
        if (buflen > 0)
            buf[buflen - 1]= '\0';

[gfwilliams]

Thanks for this. Looks like strncpy fills the whole array with 0s up to buflen as well, so it's been wasting a lot of CPU time when I've used it in the wrong place.