WebAssembly / WebAssembly/wabt

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

Open
#2,749 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
8.1k
Forks
827
Avg merge
4d 6h
Merged PRs (30d)
18

Description

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 countGetNumParamsAndLocals(). 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

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with src/binary-reader-ir.cc: OnLocalName and src/ir.cc: MakeTypeBindingReverseMapping, then reproduce the issue with the supplied 43-byte poc.wasm through wasm2wat. Validate local_index before storing the binding and retain the reverse-mapping bounds check as defense in depth. Done means the crafted module is rejected safely and the default wasm2wat, wasm2c, and wasm-decompile paths no longer abort or write out of bounds.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers, devtools, security
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.