[wasm] Errors in names section do not fail the whole module.

src/wasm/module-decoder.cc

 // Copyright 2015 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.
 
 #include "src/wasm/module-decoder.h"
 
 #include "src/base/functional.h"
 #include "src/base/platform/platform.h"
 #include "src/flags.h"
 #include "src/macro-assembler.h"
@@ skipping 591 matching lines @@
             0                // source_size
         });
         WasmDataSegment* segment = &module->data_segments.back();
         DecodeDataSegmentInModule(module, segment);
       }
       section_iter.advance();
     }
 
     // ===== Name section ====================================================
     if (section_iter.section_code() == kNameSectionCode) {
-      uint32_t functions_count = consume_u32v("functions count");
+      // Use an inner decoder so that errors don't fail the outer decoder.
+      Decoder inner(start_, pc_, end_);

[ahaas, 2017/01/24 12:40:14]

Can we tell a developer in some way that the names section is broken? I could
imagine that hiding problems in the names section could cause an unpleasant
debugging experience.

[titzer, 2017/01/24 12:42:56]

Added a TODO.

+      uint32_t functions_count = inner.consume_u32v("functions count");
 
-      for (uint32_t i = 0; ok() && i < functions_count; ++i) {
+      for (uint32_t i = 0; inner.ok() && i < functions_count; ++i) {
         uint32_t function_name_length = 0;
-        uint32_t name_offset = consume_string(&function_name_length, false);
+        uint32_t name_offset =
+            consume_string(inner, &function_name_length, false);
         uint32_t func_index = i;
         if (func_index < module->functions.size()) {
           module->functions[func_index].name_offset = name_offset;
           module->functions[func_index].name_length = function_name_length;
         }
 
-        uint32_t local_names_count = consume_u32v("local names count");
+        uint32_t local_names_count = inner.consume_u32v("local names count");
         for (uint32_t j = 0; ok() && j < local_names_count; j++) {
-          skip_string();
+          uint32_t length = inner.consume_u32v("string length");
+          inner.consume_bytes(length, "string");
         }
       }
+      // Skip the whole names section in the outer decoder.
+      consume_bytes(section_iter.payload_length(), nullptr);
       section_iter.advance();
     }
 
     // ===== Remaining sections ==============================================
     if (section_iter.more() && ok()) {
       error(pc(), pc(), "unexpected section: %s",
             SectionName(section_iter.section_code()));
     }
 
     if (ok()) {
@@ skipping 165 matching lines @@
       char* buffer = new char[len];
       strncpy(buffer, raw, len);
       buffer[len - 1] = 0;
 
       // Copy error code and location.
       result_.MoveFrom(result);
       result_.error_msg.reset(buffer);
     }
   }
 
+  uint32_t consume_string(uint32_t* length, bool validate_utf8) {
+    return consume_string(*this, length, validate_utf8);
+  }
+
   // Reads a length-prefixed string, checking that it is within bounds. Returns
   // the offset of the string, and the length as an out parameter.
-  uint32_t consume_string(uint32_t* length, bool validate_utf8) {
-    *length = consume_u32v("string length");
-    uint32_t offset = pc_offset();
-    const byte* string_start = pc_;
+  uint32_t consume_string(Decoder& decoder, uint32_t* length,
+                          bool validate_utf8) {
+    *length = decoder.consume_u32v("string length");
+    uint32_t offset = decoder.pc_offset();
+    const byte* string_start = decoder.pc();
     // Consume bytes before validation to guarantee that the string is not oob.
-    if (*length > 0) consume_bytes(*length, "string");
-    if (ok() && validate_utf8 &&
+    if (*length > 0) decoder.consume_bytes(*length, "string");
+    if (decoder.ok() && validate_utf8 &&
         !unibrow::Utf8::Validate(string_start, *length)) {
-      error(string_start, "no valid UTF-8 string");
+      decoder.error(string_start, "no valid UTF-8 string");
     }
     return offset;
   }
 
-  // Skips over a length-prefixed string, but checks that it is within bounds.
-  void skip_string() {
-    uint32_t length = consume_u32v("string length");
-    consume_bytes(length, "string");
-  }
-
   uint32_t consume_sig_index(WasmModule* module, FunctionSig** sig) {
     const byte* pos = pc_;
     uint32_t sig_index = consume_u32v("signature index");
     if (sig_index >= module->signatures.size()) {
       error(pos, pos, "signature index %u out of bounds (%d signatures)",
             sig_index, static_cast<int>(module->signatures.size()));
       *sig = nullptr;
       return 0;
     }
     *sig = module->signatures[sig_index];
@@ skipping 413 matching lines @@
     table.push_back(std::move(func_asm_offsets));
   }
   if (decoder.more()) decoder.error("unexpected additional bytes");
 
   return decoder.toResult(std::move(table));
 }
 
 }  // namespace wasm
 }  // namespace internal
 }  // namespace v8