New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A Heap-buffer-overflow problem was discovered in function wasm::SExpressionParser::skipWhitespace() in wasm-s-parser.cpp in wasm. #1863

Closed
wcventure opened this Issue Jan 12, 2019 · 1 comment

Comments

Projects
None yet
2 participants
@wcventure
Copy link

wcventure commented Jan 12, 2019

Hi, there.

A Heap-buffer-overflow problem was discovered in function wasm::SExpressionParser::skipWhitespace() in wasm-s-parser.cpp in wasm. A crafted wasm input can cause segment faults and I have confirmed them with address sanitizer too.

Here are the POC files. Please use "./wasm2js $POC" to reproduce the error.
POC.zip

The ASAN dumps the stack trace as follows:

=================================================================
==17910==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6020000001f7 at pc 0x00000188f694 bp 0x7ffed69f49b0 sp 0x7ffed69f49a8
READ of size 1 at 0x6020000001f7 thread T0
    #0 0x188f693 in wasm::SExpressionParser::skipWhitespace() /binaryen/src/wasm/wasm-s-parser.cpp:175:20
    #1 0x188b355 in wasm::SExpressionParser::parse() /binaryen/src/wasm/wasm-s-parser.cpp:121:5
    #2 0x188aecf in wasm::SExpressionParser::SExpressionParser(char*) /binaryen/src/wasm/wasm-s-parser.cpp:112:12
    #3 0x64b1a6 in std::unique_ptr<wasm::SExpressionParser, std::default_delete<wasm::SExpressionParser> > wasm::make_unique<wasm::SExpressionParser, char*>(char*&&) /binaryen/src/support/utilities.h:58:35
    #4 0x64b1a6 in main /binaryen/src/tools/wasm2js.cpp:87
    #5 0x7f18ad0d782f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #6 0x50ccb8 in _start (/binaryen/build/bin/wasm2js+0x50ccb8)

0x6020000001f7 is located 0 bytes to the right of 7-byte region [0x6020000001f0,0x6020000001f7)
allocated by thread T0 here:
    #0 0x6051b0 in operator new(unsigned long) /home/wencheng/Documents/CLib/llvm-6.0.1/projects/compiler-rt/lib/asan/asan_new_delete.cc:92
    #1 0x1b56f57 in __gnu_cxx::new_allocator<char>::allocate(unsigned long, void const*) /usr/lib/gcc/x86_64-linux-gnu/7.3.0/../../../../include/c++/7.3.0/ext/new_allocator.h:111:27
    #2 0x1b56f57 in std::allocator_traits<std::allocator<char> >::allocate(std::allocator<char>&, unsigned long) /usr/lib/gcc/x86_64-linux-gnu/7.3.0/../../../../include/c++/7.3.0/bits/alloc_traits.h:436
    #3 0x1b56f57 in std::_Vector_base<char, std::allocator<char> >::_M_allocate(unsigned long) /usr/lib/gcc/x86_64-linux-gnu/7.3.0/../../../../include/c++/7.3.0/bits/stl_vector.h:172
    #4 0x1b56f57 in std::_Vector_base<char, std::allocator<char> >::_M_create_storage(unsigned long) /usr/lib/gcc/x86_64-linux-gnu/7.3.0/../../../../include/c++/7.3.0/bits/stl_vector.h:187
    #5 0x1b56f57 in std::_Vector_base<char, std::allocator<char> >::_Vector_base(unsigned long, std::allocator<char> const&) /usr/lib/gcc/x86_64-linux-gnu/7.3.0/../../../../include/c++/7.3.0/bits/stl_vector.h:138
    #6 0x1b56f57 in std::vector<char, std::allocator<char> >::vector(unsigned long, char const&, std::allocator<char> const&) /usr/lib/gcc/x86_64-linux-gnu/7.3.0/../../../../include/c++/7.3.0/bits/stl_vector.h:297
    #7 0x1b56f57 in std::vector<char, std::allocator<char> > wasm::read_file<std::vector<char, std::allocator<char> > >(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, wasm::Flags::BinaryOption, wasm::Flags::DebugOption) /binaryen/src/support/file.cpp:42
    #8 0x64ae18 in main /binaryen/src/tools/wasm2js.cpp:85:11
    #9 0x7f18ad0d782f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

SUMMARY: AddressSanitizer: heap-buffer-overflow /binaryen/src/wasm/wasm-s-parser.cpp:175:20 in wasm::SExpressionParser::skipWhitespace()
Shadow bytes around the buggy address:
  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 fd fa fa fa fd fd fa fa 02 fa fa fa 00 fa
  0x0c047fff8010: fa fa 06 fa fa fa 04 fa fa fa 00 02 fa fa 00 02
  0x0c047fff8020: fa fa 06 fa fa fa 02 fa fa fa 02 fa fa fa 02 fa
=>0x0c047fff8030: fa fa 04 fa fa fa 02 fa fa fa 02 fa fa fa[07]fa
  0x0c047fff8040: fa fa 00 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
  0x0c047fff8060: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8070: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8080: 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
==17910==ABORTING

$ git log

commit 45714b5fc6cf14c112bc4f188aca427464ab69d8
Author: Alon Zakai <alonzakai@gmail.com>
Date:   Thu Jan 10 19:31:20 2019 -0800

    Compare binaryen fuzz-exec to JS VMs (#1856)

    The main fuzz_opt.py script compares JS VMs, and separately runs binaryen's fuzz-exec that compares the binaryen interpreter to itself (before and after opts). This PR lets us directly compare binaryen's interpreter output to JS VMs. This found a bunch of minor things we can do better on both sides, giving more fuzz coverage.

    To enable this, a bunch of tiny fixes were needed:

    *    Add --fuzz-exec-before which is like --fuzz-exec but just runs the code before opts are run, instead of before and after.
    *    Normalize double printing (so JS and C++ print comparable things). This includes negative zero in JS, which we never printed properly til now.
    *    Various improvements to how we print fuzz-exec logging - remove unuseful things, and normalize the others across JS and C++.
    *    Properly legalize the wasm when --emit-js-wrapper (i.e., we will run the code from JS), and use that in the JS wrapper code.
@kripken

This comment has been minimized.

Copy link
Member

kripken commented Jan 15, 2019

Thanks! Fix in #1869.

kripken added a commit that referenced this issue Jan 16, 2019

Misc minor ASAN fixes (#1869)
* handle end of input in skipWhitespace in s-parser. fixes #1863

* ignore debug locations when not in a function ; fixes #1867

* error properly on invalid user section sizes ; fixes #1866

* throw a proper error on invalid call offsets in binary reading ; fixes #1865
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment