Skip to content

Commit 2a8dc8b

Browse files
authored
Fix out-of-bounds write in MakeTypeBindingReverseMapping (#2796)
ASan, release build, `wasm2wat` on a crafted module: ==…==ERROR: AddressSanitizer: heap-buffer-overflow #0 wabt::MakeTypeBindingReverseMapping(...) ir.cc #1 wabt::ApplyNames(wabt::Module*) #2 ProgramMain ... located 1535 bytes after 24-byte region Found while going over the name-section handlers. The local subsection stores a (local index, name) pair per entry. `OnLocalNameLocalCount` checks the entry count against the function's param+local count, but the per-entry local index in `OnLocalName` is never range-checked before it becomes a `Binding`. `MakeTypeBindingReverseMapping` sizes the reverse map to that local count and writes each name at `binding.index`, guarded only by an `assert`. A name whose index sits past the function's locals writes off the end of the vector: a debug build trips the assert, a release build corrupts the heap. Reached from an untrusted `.wasm` through `wasm2wat` and `wasm2c` (both call `ApplyNames` and read debug names by default), on an otherwise valid module. Skip entries that land outside the mapping. Valid modules only name real locals, so their output does not change. The added test names local index 5 of a one-param function; before the fix `wasm2wat` aborts, after it prints the module with the stray name dropped.
1 parent 33328a0 commit 2a8dc8b

2 files changed

Lines changed: 41 additions & 2 deletions

File tree

src/ir.cc

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -605,8 +605,13 @@ void MakeTypeBindingReverseMapping(
605605
out_reverse_mapping->clear();
606606
out_reverse_mapping->resize(num_types);
607607
for (const auto& [name, binding] : bindings) {
608-
assert(static_cast<size_t>(binding.index) < out_reverse_mapping->size());
609-
(*out_reverse_mapping)[binding.index] = name;
608+
// A binding index can come straight from the name section's local
609+
// subsection, which is not otherwise range-checked against the function's
610+
// local count, so skip entries that fall outside the mapping instead of
611+
// writing past it.
612+
if (static_cast<size_t>(binding.index) < out_reverse_mapping->size()) {
613+
(*out_reverse_mapping)[binding.index] = name;
614+
}
610615
}
611616
}
612617

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
;;; TOOL: run-gen-wasm
2+
;; The name section's local subsection can carry a local index that is out of
3+
;; range for the function it names. The reader accepts such an entry, so
4+
;; MakeTypeBindingReverseMapping used to write past the reverse-mapping vector
5+
;; (heap overflow in a release build, assert in a debug build). The out of range
6+
;; name is now dropped and wasm2wat prints the module unchanged.
7+
magic
8+
version
9+
section(TYPE) {
10+
count[1]
11+
function params[1] i32 results[0]
12+
}
13+
section(FUNCTION) {
14+
count[1]
15+
type[0]
16+
}
17+
section(CODE) {
18+
count[1]
19+
func { locals[0] }
20+
}
21+
section("name") {
22+
section(NAME_LOCALS) {
23+
func_count[1]
24+
index[0]
25+
local_count[1]
26+
index[5] ;; function has a single param (index 0), so 5 is invalid
27+
str("BAD")
28+
}
29+
}
30+
(;; STDOUT ;;;
31+
(module
32+
(type (;0;) (func (param i32)))
33+
(func (;0;) (type 0) (param i32)))
34+
;;; STDOUT ;;)

0 commit comments

Comments
 (0)