Fix heap-based buffer overflow in GetReturnCallDropKeepCount by adding checks in OnReturnCallExpr

[PGZXB]

Fixes #2557

Root Cause

The interpreter’s OnReturnCallExpr uses func_index to index into its internal func_types_ vector without proper bounds checking. When a return_call instruction references a function index that has not yet been added to func_types_, an invalid FuncType reference is passed to GetReturnCallDropKeepCount, leading to a heap-based buffer overflow when accessing func_type.params.

Fix

Add explicit bounds checking in OnReturnCallExpr to ensure func_index < func_types_.size() before accessing func_types_. This prevents invalid FuncType references and eliminates the out-of-bounds read that caused the heap-buffer-overflow.

Verification Before Fix

Running the PoC triggers AddressSanitizer error:

INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 3125788798
INFO: Loaded 1 modules   (710 inline 8-bit counters): 710 [0x55cf916d68c0, 0x55cf916d6b86), 
INFO: Loaded 1 PC tables (710 PCs): 710 [0x55cf916d6b88,0x55cf916d97e8), 
/new_issue/read_binary_interp_fuzzer: Running 1 inputs 1 time(s) each.
Running: /new_issue/crash
=================================================================
==19727==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x50d0000001b8 at pc 0x55cf913989e6 bp 0x7ffd75a17ec0 sp 0x7ffd75a17eb8
READ of size 8 at 0x50d0000001b8 thread T0
    #0 0x55cf913989e5 in std::vector<wabt::Type, std::allocator<wabt::Type>>::size() const /usr/lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/stl_vector.h:916:40
    #1 0x55cf91382a92 in wabt::interp::(anonymous namespace)::BinaryReaderInterp::GetReturnCallDropKeepCount(wabt::interp::FuncType const&, unsigned int, unsigned int*, unsigned int*) /src/wabt/src/interp/binary-reader-interp.cc:462:58
    #2 0x55cf913775ed in wabt::interp::(anonymous namespace)::BinaryReaderInterp::OnReturnCallExpr(unsigned int) /src/wabt/src/interp/binary-reader-interp.cc:1249:3
    #3 0x55cf9148cb67 in wabt::(anonymous namespace)::BinaryReader::ReadInstructions(unsigned long, char const*) /src/wabt/src/binary-reader.cc:1008:9
    #4 0x55cf9149912f in wabt::(anonymous namespace)::BinaryReader::ReadFunctionBody(unsigned long) /src/wabt/src/binary-reader.cc:741:3
    #5 0x55cf91474eb9 in wabt::(anonymous namespace)::BinaryReader::ReadCodeSection(unsigned long) /src/wabt/src/binary-reader.cc:3044:7
    #6 0x55cf9146acee in wabt::(anonymous namespace)::BinaryReader::ReadSections(wabt::(anonymous namespace)::BinaryReader::ReadSectionsOptions const&) /src/wabt/src/binary-reader.cc:3197:26
    #7 0x55cf914690a9 in wabt::(anonymous namespace)::BinaryReader::ReadModule(wabt::(anonymous namespace)::BinaryReader::ReadModuleOptions const&) /src/wabt/src/binary-reader.cc:3271:3
    #8 0x55cf91468379 in wabt::ReadBinary(void const*, unsigned long, wabt::BinaryReaderDelegate*, wabt::ReadBinaryOptions const&) /src/wabt/src/binary-reader.cc:3293:17
    #9 0x55cf913648be in wabt::interp::ReadBinaryInterp(std::basic_string_view<char, std::char_traits<char>>, void const*, unsigned long, wabt::ReadBinaryOptions const&, std::vector<wabt::Error, std::allocator<wabt::Error>>*, wabt::interp::ModuleDesc*) /src/wabt/src/interp/binary-reader-interp.cc:1821:10
    #10 0x55cf9134293a in LLVMFuzzerTestOneInput /new_issue/read_binary_interp_fuzzer.cc:39:3
    #11 0x55cf911f6990 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:614:13
    #12 0x55cf911e1c05 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:327:6
    #13 0x55cf911e769f in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:862:9
    #14 0x55cf91212942 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
    #15 0x7f33ece8b082 in __libc_start_main /build/glibc-B3wQXB/glibc-2.31/csu/../csu/libc-start.c:308:16
    #16 0x55cf911d9ded in _start (/new_issue/read_binary_interp_fuzzer+0xdeded)

0x50d0000001b8 is located 24 bytes after 144-byte region [0x50d000000110,0x50d0000001a0)
allocated by thread T0 here:
    #0 0x55cf9133fb2d in operator new(unsigned long) /src/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:86:3
    #1 0x55cf91396108 in __gnu_cxx::new_allocator<wabt::interp::FuncType>::allocate(unsigned long, void const*) /usr/lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/ext/new_allocator.h:114:27
    #2 0x55cf913960b0 in std::allocator_traits<std::allocator<wabt::interp::FuncType>>::allocate(std::allocator<wabt::interp::FuncType>&, unsigned long) /usr/lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/alloc_traits.h:443:20
    #3 0x55cf91395e2f in std::_Vector_base<wabt::interp::FuncType, std::allocator<wabt::interp::FuncType>>::_M_allocate(unsigned long) /usr/lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/stl_vector.h:343:20
    #4 0x55cf9139b366 in void std::vector<wabt::interp::FuncType, std::allocator<wabt::interp::FuncType>>::_M_realloc_insert<wabt::interp::FuncType const&>(__gnu_cxx::__normal_iterator<wabt::interp::FuncType*, std::vector<wabt::interp::FuncType, std::allocator<wabt::interp::FuncType>>>, wabt::interp::FuncType const&) /usr/lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/vector.tcc:440:33
    #5 0x55cf913996ec in std::vector<wabt::interp::FuncType, std::allocator<wabt::interp::FuncType>>::push_back(wabt::interp::FuncType const&) /usr/lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/stl_vector.h:1195:4
    #6 0x55cf91367790 in wabt::interp::(anonymous namespace)::BinaryReaderInterp::OnFunction(unsigned int, unsigned int) /src/wabt/src/interp/binary-reader-interp.cc:627:15
    #7 0x55cf9146f3e4 in wabt::(anonymous namespace)::BinaryReader::ReadFunctionSection(unsigned long) /src/wabt/src/binary-reader.cc:2815:5
    #8 0x55cf9146aa39 in wabt::(anonymous namespace)::BinaryReader::ReadSections(wabt::(anonymous namespace)::BinaryReader::ReadSectionsOptions const&) /src/wabt/src/binary-reader.cc:3169:26
    #9 0x55cf914690a9 in wabt::(anonymous namespace)::BinaryReader::ReadModule(wabt::(anonymous namespace)::BinaryReader::ReadModuleOptions const&) /src/wabt/src/binary-reader.cc:3271:3
    #10 0x55cf91468379 in wabt::ReadBinary(void const*, unsigned long, wabt::BinaryReaderDelegate*, wabt::ReadBinaryOptions const&) /src/wabt/src/binary-reader.cc:3293:17
    #11 0x55cf913648be in wabt::interp::ReadBinaryInterp(std::basic_string_view<char, std::char_traits<char>>, void const*, unsigned long, wabt::ReadBinaryOptions const&, std::vector<wabt::Error, std::allocator<wabt::Error>>*, wabt::interp::ModuleDesc*) /src/wabt/src/interp/binary-reader-interp.cc:1821:10
    #12 0x55cf9134293a in LLVMFuzzerTestOneInput /new_issue/read_binary_interp_fuzzer.cc:39:3
    #13 0x55cf911f6990 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:614:13
    #14 0x55cf911e1c05 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:327:6
    #15 0x55cf911e769f in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:862:9
    #16 0x55cf91212942 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
    #17 0x7f33ece8b082 in __libc_start_main /build/glibc-B3wQXB/glibc-2.31/csu/../csu/libc-start.c:308:16

SUMMARY: AddressSanitizer: heap-buffer-overflow /src/wabt/src/interp/binary-reader-interp.cc:462:58 in wabt::interp::(anonymous namespace)::BinaryReaderInterp::GetReturnCallDropKeepCount(wabt::interp::FuncType const&, unsigned int, unsigned int*, unsigned int*)
Shadow bytes around the buggy address:
  0x50cfffffff00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x50cfffffff80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x50d000000000: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x50d000000080: fd fd fd fd fd fd fd fd fd fd fa fa fa fa fa fa
  0x50d000000100: fa fa 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x50d000000180: 00 00 00 00 fa fa fa[fa]fa fa fa fa fa fa fa fa
  0x50d000000200: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x50d000000280: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x50d000000300: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x50d000000380: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x50d000000400: 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
==19727==ABORTING

Verification After Fix

Running the PoC:

INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 3026971333
INFO: Loaded 1 modules   (710 inline 8-bit counters): 710 [0x55e8358158c0, 0x55e835815b86), 
INFO: Loaded 1 PC tables (710 PCs): 710 [0x55e835815b88,0x55e8358187e8), 
/new_issue/read_binary_interp_fuzzer: Running 1 inputs 1 time(s) each.
Running: /new_issue/crash
Executed /new_issue/crash in 0 ms
***
*** NOTE: fuzzing was not performed, you have only
***       executed the target code on a fixed set of inputs.
***

[PGZXB]

Hi @sbc100,

I’ve submitted this PR to address #2557. The CI checks have completed successfully.

I’m not sure who would be the appropriate reviewer, and I hope I’m not disturbing you. Could you please review the changes, or help request a review from the right person?