[wasm] Fix max index in function names section

Fixed: chromium:1341180
Change-Id: Ib475310b18c31e5e3e0fc5e52dab736ebb6ac55a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3738745Reviewed-by: Andreas Haas <ahaas@chromium.org>
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81527}

src/wasm/module-decoder.cc

@@ -290,53 +290,18 @@ bool FindNameSection(Decoder* decoder) {
   return true;
 }
 
-}  // namespace
+enum EmptyNames : bool { kAllowEmptyNames, kSkipEmptyNames };
 
-void DecodeFunctionNames(const byte* module_start, const byte* module_end,
-                         NameMap& names) {
-  Decoder decoder(module_start, module_end);
-  if (FindNameSection(&decoder)) {
-    while (decoder.ok() && decoder.more()) {
-      uint8_t name_type = decoder.consume_u8("name type");
-      if (name_type & 0x80) break;  // no varuint7
-
-      uint32_t name_payload_len = decoder.consume_u32v("name payload length");
-      if (!decoder.checkAvailable(name_payload_len)) break;
-
-      if (name_type != NameSectionKindCode::kFunctionCode) {
-        decoder.consume_bytes(name_payload_len, "name subsection payload");
-        continue;
-      }
-      uint32_t functions_count = decoder.consume_u32v("functions count");
-
-      for (; decoder.ok() && functions_count > 0; --functions_count) {
-        uint32_t function_index = decoder.consume_u32v("function index");
-        WireBytesRef name =
-            consume_string(&decoder, StringValidation::kNone, "function name");
-
-        // Be lenient with errors in the name section: Ignore non-UTF8 names.
-        // You can even assign to the same function multiple times (last valid
-        // one wins).
-        if (decoder.ok() && validate_utf8(&decoder, name)) {
-          names.Put(function_index, name);
-        }
-      }
-    }
-  }
-  names.FinishInitialization();
-}
-
-namespace {
-
-void DecodeNameMap(NameMap& target, Decoder& decoder) {
+void DecodeNameMap(NameMap& target, Decoder& decoder,
+                   EmptyNames empty_names = kSkipEmptyNames) {
   uint32_t count = decoder.consume_u32v("names count");
   for (uint32_t i = 0; i < count; i++) {
     uint32_t index = decoder.consume_u32v("index");
     WireBytesRef name =
         consume_string(&decoder, StringValidation::kNone, "name");
     if (!decoder.ok()) break;
-    if (index > kMaxInt) continue;
-    if (name.is_empty()) continue;  // Empty names are useless.
+    if (index > NameMap::kMaxKey) continue;
+    if (empty_names == kSkipEmptyNames && name.is_empty()) continue;
     if (!validate_utf8(&decoder, name)) continue;
     target.Put(index, name);
   }
@@ -347,7 +312,7 @@ void DecodeIndirectNameMap(IndirectNameMap& target, Decoder& decoder) {
   uint32_t outer_count = decoder.consume_u32v("outer count");
   for (uint32_t i = 0; i < outer_count; ++i) {
     uint32_t outer_index = decoder.consume_u32v("outer index");
-    if (outer_index > kMaxInt) continue;
+    if (outer_index > IndirectNameMap::kMaxKey) continue;
     NameMap names;
     uint32_t inner_count = decoder.consume_u32v("inner count");
     for (uint32_t k = 0; k < inner_count; ++k) {
@@ -355,7 +320,7 @@ void DecodeIndirectNameMap(IndirectNameMap& target, Decoder& decoder) {
       WireBytesRef name =
           consume_string(&decoder, StringValidation::kNone, "name");
       if (!decoder.ok()) break;
-      if (inner_index > kMaxInt) continue;
+      if (inner_index > NameMap::kMaxKey) continue;
       if (name.is_empty()) continue;  // Empty names are useless.
       if (!validate_utf8(&decoder, name)) continue;
       names.Put(inner_index, name);
@@ -368,6 +333,27 @@ void DecodeIndirectNameMap(IndirectNameMap& target, Decoder& decoder) {
 
 }  // namespace
 
+void DecodeFunctionNames(const byte* module_start, const byte* module_end,
+                         NameMap& names) {
+  Decoder decoder(module_start, module_end);
+  if (FindNameSection(&decoder)) {
+    while (decoder.ok() && decoder.more()) {
+      uint8_t name_type = decoder.consume_u8("name type");
+      if (name_type & 0x80) break;  // no varuint7
+
+      uint32_t name_payload_len = decoder.consume_u32v("name payload length");
+      if (!decoder.checkAvailable(name_payload_len)) break;
+
+      if (name_type != NameSectionKindCode::kFunctionCode) {
+        decoder.consume_bytes(name_payload_len, "name subsection payload");
+        continue;
+      }
+      // We need to allow empty function names for spec-conformant stack traces.
+      DecodeNameMap(names, decoder, kAllowEmptyNames);
+    }
+  }
+}
+
 DecodedNameSection::DecodedNameSection(base::Vector<const uint8_t> wire_bytes,
                                        WireBytesRef name_section) {
   if (name_section.is_empty()) return;  // No name section.
@@ -388,33 +374,46 @@ DecodedNameSection::DecodedNameSection(base::Vector<const uint8_t> wire_bytes,
         decoder.consume_bytes(name_payload_len);
         break;
       case kLocalCode:
+        static_assert(kV8MaxWasmFunctions <= IndirectNameMap::kMaxKey);
+        static_assert(kV8MaxWasmFunctionLocals <= NameMap::kMaxKey);
         DecodeIndirectNameMap(local_names_, decoder);
         break;
       case kLabelCode:
+        static_assert(kV8MaxWasmFunctions <= IndirectNameMap::kMaxKey);
+        static_assert(kV8MaxWasmFunctionSize <= NameMap::kMaxKey);
         DecodeIndirectNameMap(label_names_, decoder);
         break;
       case kTypeCode:
+        static_assert(kV8MaxWasmTypes <= NameMap::kMaxKey);
         DecodeNameMap(type_names_, decoder);
         break;
       case kTableCode:
+        static_assert(kV8MaxWasmTables <= NameMap::kMaxKey);
         DecodeNameMap(table_names_, decoder);
         break;
       case kMemoryCode:
+        static_assert(kV8MaxWasmMemories <= NameMap::kMaxKey);
         DecodeNameMap(memory_names_, decoder);
         break;
       case kGlobalCode:
+        static_assert(kV8MaxWasmGlobals <= NameMap::kMaxKey);
         DecodeNameMap(global_names_, decoder);
         break;
       case kElementSegmentCode:
+        static_assert(kV8MaxWasmTableInitEntries <= NameMap::kMaxKey);
         DecodeNameMap(element_segment_names_, decoder);
         break;
       case kDataSegmentCode:
+        static_assert(kV8MaxWasmDataSegments <= NameMap::kMaxKey);
         DecodeNameMap(data_segment_names_, decoder);
         break;
       case kFieldCode:
+        static_assert(kV8MaxWasmTypes <= IndirectNameMap::kMaxKey);
+        static_assert(kV8MaxWasmStructFields <= NameMap::kMaxKey);
         DecodeIndirectNameMap(field_names_, decoder);
         break;
       case kTagCode:
+        static_assert(kV8MaxWasmTags <= NameMap::kMaxKey);
         DecodeNameMap(tag_names_, decoder);
         break;
     }

src/wasm/wasm-module.h

@@ -218,6 +218,12 @@ enum ModuleOrigin : uint8_t {
 template <class Value>
 class AdaptiveMap {
  public:
+  // The technical limitation here is that index+1 must not overflow. Since
+  // we have significantly lower maximums on anything that can be named,
+  // we can have a tighter limit here to reject useless entries early.
+  static constexpr uint32_t kMaxKey = 10'000'000;
+  static_assert(kMaxKey < std::numeric_limits<uint32_t>::max());
+
   AdaptiveMap() : map_(new MapType()) {}
 
   explicit AdaptiveMap(const AdaptiveMap&) = delete;
@@ -238,11 +244,13 @@ class AdaptiveMap {
 
   void Put(uint32_t key, const Value& value) {
     DCHECK(mode_ == kInitializing);
+    DCHECK_LE(key, kMaxKey);
     map_->insert(std::make_pair(key, value));
   }
 
   void Put(uint32_t key, Value&& value) {
     DCHECK(mode_ == kInitializing);
+    DCHECK_LE(key, kMaxKey);
     map_->insert(std::make_pair(key, std::move(value)));
   }
 

test/mjsunit/regress/wasm/regress-crbug-1341180.js

+// Copyright 2022 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+let bytes = Uint8Array.from([
+  0x00, 0x61, 0x73, 0x6d,  // wasm magic
+  0x01, 0x00, 0x00, 0x00,  // wasm version
+
+  0x01, 0x04,              // Type section, 4 bytes
+  0x01,                    // types count 1
+  0x60, 0x00, 0x00,        // kind: func, no params, no returns
+
+  0x03, 0x02,              // Function section, 2 bytes
+  0x01,                    // functions count 1
+  0x00,                    // func0: type0 (result i32)
+
+  0x0a, 0x02,              // Code section, 2 bytes
+  0x01,                    // functions count 1
+                           // function #0
+  0x00,                    // body size 0 (which is invalid, which causes
+                           // lazy decoding of the name section in order to
+                           // produce an error message)
+
+  0x00, 0x10,              // Unknown section, 16 bytes
+  0x04,                    // section name length: 4
+  0x6e, 0x61, 0x6d, 0x65,  // section name: name
+  0x01,                    // name type: function
+  0x09,                    // payload length: 9
+  0x02,                    // names count 2
+  0xff, 0xff, 0xff, 0xff, 0x0f, 0x00,  // index 4294967295 name length: 0
+  0x00, 0x00,              // index 0 name length: 0
+]);
+
+assertThrows(
+    () => new WebAssembly.Instance(new WebAssembly.Module(bytes)),
+    WebAssembly.CompileError);