[Segmentation fault] Out-of-bounds read in wasm::WATParser::makeArrayInitElem

Hi,

I identified an out-of-bounds data read bug when fuzzing the wasm-opt tool.

Steps to reproduce:
Test file: wasm-oob-read.zip
Command: ./wasm-opt ./wasm-oob-read.wasm

GDB:

gdb --args ./wasm-opt ./wasm-oob-read.wasm 
GNU gdb (Ubuntu 12.1-0ubuntu1~22.04.2) 12.1

Reading symbols from ./wasm-opt...
(gdb) r
Starting program: /home/sofi/wasm-opt ./wasm-oob-read.wasm
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".

Program received signal SIGSEGV, Segmentation fault.
0x00005555565cf5c5 in wasm::WATParser::makeArrayInitElem<wasm::WATParser::ParseDefsCtx> (ctx=..., pos=110, annotations=std::vector of length 0, capacity 0) at /home/sofi/fuzzing/binaryen/binaryen/src/parser/parsers.h:2332
2332	  return ctx.makeArrayInitElem(pos, annotations, *type, *elem);
(gdb) bt
#0  0x00005555565cf5c5 in wasm::WATParser::makeArrayInitElem<wasm::WATParser::ParseDefsCtx> (ctx=..., pos=110, annotations=std::vector of length 0, capacity 0) at /home/sofi/fuzzing/binaryen/binaryen/src/parser/parsers.h:2332
#1  0x0000555556598d1b in wasm::WATParser::plaininstr<wasm::WATParser::ParseDefsCtx> (ctx=..., annotations=std::vector of length 0, capacity 0) at /home/sofi/fuzzing/binaryen/binaryen/src/gen-s-parser.inc:75
#2  0x0000555556595eb4 in wasm::WATParser::instr<wasm::WATParser::ParseDefsCtx> (ctx=...) at /home/sofi/fuzzing/binaryen/binaryen/src/parser/parsers.h:938
#3  0x00005555565974e4 in wasm::WATParser::instrs<wasm::WATParser::ParseDefsCtx> (ctx=...) at /home/sofi/fuzzing/binaryen/binaryen/src/parser/parsers.h:1018
#4  0x000055555658f441 in wasm::WATParser::func<wasm::WATParser::ParseDefsCtx> (ctx=...) at /home/sofi/fuzzing/binaryen/binaryen/src/parser/parsers.h:2986
#5  0x000055555657deb1 in wasm::WATParser::parseDefinitions (decls=..., input=..., typeIndices=std::unordered_map with 0 elements, types=std::vector of length 1, capacity 1 = {...}, implicitTypes=std::unordered_map with 2 elements = {...}, 
    typeNames=std::unordered_map with 0 elements) at /home/sofi/fuzzing/binaryen/binaryen/src/parser/parse-5-defs.cpp:51
#6  0x00005555562f6e1c in wasm::WATParser::(anonymous namespace)::doParseModule (wasm=..., input=..., allowExtra=false) at /home/sofi/fuzzing/binaryen/binaryen/src/parser/wat-parser.cpp:113
#7  0x00005555562f7185 in wasm::WATParser::parseModule (wasm=..., in="(func      return", ' ' <repeats 49 times>, "i32.or      i8x16.ge_u   f64.const   5      array.init_elem", ' ' <repeats 15 times>, "0", ' ' <repeats 43 times>, "i64.le_u        "...)
    at /home/sofi/fuzzing/binaryen/binaryen/src/parser/wat-parser.cpp:126
#8  0x00005555560e0c4b in wasm::readTextData (input="(func      return", ' ' <repeats 49 times>, "i32.or      i8x16.ge_u   f64.const   5      array.init_elem", ' ' <repeats 15 times>, "0", ' ' <repeats 43 times>, "i64.le_u        "..., wasm=..., 
    profile=wasm::IRProfile::Normal) at /home/sofi/fuzzing/binaryen/binaryen/src/wasm/wasm-io.cpp:38
#9  0x00005555560e0daf in wasm::ModuleReader::readText (this=0x7fffffffc4a0, filename="./wasm-oob-read.wasm", wasm=...) at /home/sofi/fuzzing/binaryen/binaryen/src/wasm/wasm-io.cpp:47
#10 0x00005555560e14ea in wasm::ModuleReader::read (this=0x7fffffffc4a0, filename="./wasm-oob-read.wasm", wasm=..., sourceMapFilename="") at /home/sofi/fuzzing/binaryen/binaryen/src/wasm/wasm-io.cpp:109
#11 0x000055555565aedd in main (argc=2, argv=0x7fffffffe108) at /home/sofi/fuzzing/binaryen/binaryen/src/tools/wasm-opt.cpp:278

Valgrind:

valgrind ./wasm-opt ./wasm-oob-read.wasm 
==3026577== Memcheck, a memory error detector
==3026577== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==3026577== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==3026577== Command: ./wasm-opt ./wasm-oob-read.wasm
==3026577== 
==3026577== Invalid read of size 8
==3026577==    at 0x11835C5: wasm::Result<wasm::Ok> wasm::WATParser::makeArrayInitElem<wasm::WATParser::ParseDefsCtx>(wasm::WATParser::ParseDefsCtx&, unsigned int, std::vector<wasm::WATParser::Annotation, std::allocator<wasm::WATParser::Annotation> > const&) (parsers.h:2332)
==3026577==    by 0x114CD1A: wasm::MaybeResult<wasm::Ok> wasm::WATParser::plaininstr<wasm::WATParser::ParseDefsCtx>(wasm::WATParser::ParseDefsCtx&, std::vector<wasm::WATParser::Annotation, std::allocator<wasm::WATParser::Annotation> > const&) (gen-s-parser.inc:75)
==3026577==    by 0x1149EB3: wasm::MaybeResult<wasm::Ok> wasm::WATParser::instr<wasm::WATParser::ParseDefsCtx>(wasm::WATParser::ParseDefsCtx&) (parsers.h:938)
==3026577==    by 0x114B4E3: wasm::Result<wasm::Ok> wasm::WATParser::instrs<wasm::WATParser::ParseDefsCtx>(wasm::WATParser::ParseDefsCtx&) (parsers.h:1018)
==3026577==    by 0x1143440: wasm::MaybeResult<wasm::Ok> wasm::WATParser::func<wasm::WATParser::ParseDefsCtx>(wasm::WATParser::ParseDefsCtx&) (parsers.h:2986)
==3026577==    by 0x1131EB0: wasm::WATParser::parseDefinitions(wasm::WATParser::ParseDeclsCtx&, wasm::WATParser::Lexer&, std::unordered_map<wasm::Name, unsigned int, std::hash<wasm::Name>, std::equal_to<wasm::Name>, std::allocator<std::pair<wasm::Name const, unsigned int> > >&, std::vector<wasm::HeapType, std::allocator<wasm::HeapType> >&, std::unordered_map<unsigned int, wasm::HeapType, std::hash<unsigned int>, std::equal_to<unsigned int>, std::allocator<std::pair<unsigned int const, wasm::HeapType> > >&, std::unordered_map<wasm::HeapType, std::unordered_map<wasm::Name, unsigned int, std::hash<wasm::Name>, std::equal_to<wasm::Name>, std::allocator<std::pair<wasm::Name const, unsigned int> > >, std::hash<wasm::HeapType>, std::equal_to<wasm::HeapType>, std::allocator<std::pair<wasm::HeapType const, std::unordered_map<wasm::Name, unsigned int, std::hash<wasm::Name>, std::equal_to<wasm::Name>, std::allocator<std::pair<wasm::Name const, unsigned int> > > > > >&) (parse-5-defs.cpp:51)
==3026577==    by 0xEAAE1B: wasm::WATParser::(anonymous namespace)::doParseModule(wasm::Module&, wasm::WATParser::Lexer&, bool) (wat-parser.cpp:113)
==3026577==    by 0xEAB184: wasm::WATParser::parseModule(wasm::Module&, std::basic_string_view<char, std::char_traits<char> >) (wat-parser.cpp:126)
==3026577==    by 0xC94C4A: wasm::readTextData(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, wasm::Module&, wasm::IRProfile) (wasm-io.cpp:38)
==3026577==    by 0xC94DAE: wasm::ModuleReader::readText(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, wasm::Module&) (wasm-io.cpp:47)
==3026577==    by 0xC954E9: wasm::ModuleReader::read(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, wasm::Module&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) (wasm-io.cpp:109)
==3026577==    by 0x20EEDC: main (wasm-opt.cpp:278)
==3026577==  Address 0x8 is not stack'd, malloc'd or (recently) free'd
==3026577== 
==3026577== 
==3026577== Process terminating with default action of signal 11 (SIGSEGV)
==3026577==  Access not within mapped region at address 0x8
==3026577==    at 0x11835C5: wasm::Result<wasm::Ok> wasm::WATParser::makeArrayInitElem<wasm::WATParser::ParseDefsCtx>(wasm::WATParser::ParseDefsCtx&, unsigned int, std::vector<wasm::WATParser::Annotation, std::allocator<wasm::WATParser::Annotation> > const&) (parsers.h:2332)
==3026577==    by 0x114CD1A: wasm::MaybeResult<wasm::Ok> wasm::WATParser::plaininstr<wasm::WATParser::ParseDefsCtx>(wasm::WATParser::ParseDefsCtx&, std::vector<wasm::WATParser::Annotation, std::allocator<wasm::WATParser::Annotation> > const&) (gen-s-parser.inc:75)
==3026577==    by 0x1149EB3: wasm::MaybeResult<wasm::Ok> wasm::WATParser::instr<wasm::WATParser::ParseDefsCtx>(wasm::WATParser::ParseDefsCtx&) (parsers.h:938)
==3026577==    by 0x114B4E3: wasm::Result<wasm::Ok> wasm::WATParser::instrs<wasm::WATParser::ParseDefsCtx>(wasm::WATParser::ParseDefsCtx&) (parsers.h:1018)
==3026577==    by 0x1143440: wasm::MaybeResult<wasm::Ok> wasm::WATParser::func<wasm::WATParser::ParseDefsCtx>(wasm::WATParser::ParseDefsCtx&) (parsers.h:2986)
==3026577==    by 0x1131EB0: wasm::WATParser::parseDefinitions(wasm::WATParser::ParseDeclsCtx&, wasm::WATParser::Lexer&, std::unordered_map<wasm::Name, unsigned int, std::hash<wasm::Name>, std::equal_to<wasm::Name>, std::allocator<std::pair<wasm::Name const, unsigned int> > >&, std::vector<wasm::HeapType, std::allocator<wasm::HeapType> >&, std::unordered_map<unsigned int, wasm::HeapType, std::hash<unsigned int>, std::equal_to<unsigned int>, std::allocator<std::pair<unsigned int const, wasm::HeapType> > >&, std::unordered_map<wasm::HeapType, std::unordered_map<wasm::Name, unsigned int, std::hash<wasm::Name>, std::equal_to<wasm::Name>, std::allocator<std::pair<wasm::Name const, unsigned int> > >, std::hash<wasm::HeapType>, std::equal_to<wasm::HeapType>, std::allocator<std::pair<wasm::HeapType const, std::unordered_map<wasm::Name, unsigned int, std::hash<wasm::Name>, std::equal_to<wasm::Name>, std::allocator<std::pair<wasm::Name const, unsigned int> > > > > >&) (parse-5-defs.cpp:51)
==3026577==    by 0xEAAE1B: wasm::WATParser::(anonymous namespace)::doParseModule(wasm::Module&, wasm::WATParser::Lexer&, bool) (wat-parser.cpp:113)
==3026577==    by 0xEAB184: wasm::WATParser::parseModule(wasm::Module&, std::basic_string_view<char, std::char_traits<char> >) (wat-parser.cpp:126)
==3026577==    by 0xC94C4A: wasm::readTextData(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, wasm::Module&, wasm::IRProfile) (wasm-io.cpp:38)
==3026577==    by 0xC94DAE: wasm::ModuleReader::readText(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, wasm::Module&) (wasm-io.cpp:47)
==3026577==    by 0xC954E9: wasm::ModuleReader::read(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, wasm::Module&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) (wasm-io.cpp:109)
==3026577==    by 0x20EEDC: main (wasm-opt.cpp:278)
==3026577==  If you believe this happened as a result of a stack
==3026577==  overflow in your program's main thread (unlikely but
==3026577==  possible), you can try to increase the size of the
==3026577==  main thread stack using the --main-stacksize= flag.
==3026577==  The main thread stack size used in this run was 8388608.
==3026577== 
==3026577== HEAP SUMMARY:
==3026577==     in use at exit: 249,343 bytes in 1,953 blocks
==3026577==   total heap usage: 3,346 allocs, 1,393 frees, 387,559 bytes allocated
==3026577== 
==3026577== LEAK SUMMARY:
==3026577==    definitely lost: 0 bytes in 0 blocks
==3026577==    indirectly lost: 0 bytes in 0 blocks
==3026577==      possibly lost: 0 bytes in 0 blocks
==3026577==    still reachable: 249,343 bytes in 1,953 blocks
==3026577==         suppressed: 0 bytes in 0 blocks
==3026577== Rerun with --leak-check=full to see details of leaked memory
==3026577== 
==3026577== For lists of detected and suppressed errors, rerun with: -s
==3026577== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
Segmentation fault

I'm not very familiar with the parsing logic here, but it looks like the issue may be a missing call to CHECK_ERR(elem) after line 2331 of file src/parser/parsers.h.

binaryen/src/parser/parsers.h

Lines 2325 to 2333 in a4f9128

    
           template<typename Ctx> 
        
           Result<> makeArrayInitElem(Ctx& ctx, 
        
                                      Index pos, 
        
                                      const std::vector<Annotation>& annotations) { 
        
             auto type = typeidx(ctx); 
        
             CHECK_ERR(type); 
        
             auto elem = elemidx(ctx); 
        
             return ctx.makeArrayInitElem(pos, annotations, *type, *elem); 
        
           }

Thanks @sofiaaberegg for the report, and yes, I think that is exactly the issue! Would you like to open a PR with it?

Yeah, will do!

	template<typename Ctx>
	Result<> makeArrayInitElem(Ctx& ctx,
	Index pos,
	const std::vector<Annotation>& annotations) {
	auto type = typeidx(ctx);
	CHECK_ERR(type);
	auto elem = elemidx(ctx);
	return ctx.makeArrayInitElem(pos, annotations, type, elem);
	}