[wasm] Enforce strict ordering of WASM module sections.

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/macro-assembler.h"
 #include "src/objects.h"
 #include "src/v8.h"
 
 #include "src/wasm/decoder.h"
 
 namespace v8 {
 namespace internal {
 namespace wasm {
 
 #if DEBUG
 #define TRACE(...)                                    \
   do {                                                \
     if (FLAG_trace_wasm_decoder) PrintF(__VA_ARGS__); \
   } while (false)
 #else
 #define TRACE(...)
 #endif
 
-
 // The main logic for decoding the bytes of a module.
 class ModuleDecoder : public Decoder {
  public:
   ModuleDecoder(Zone* zone, const byte* module_start, const byte* module_end,
                 ModuleOrigin origin)
       : Decoder(module_start, module_end), module_zone(zone), origin_(origin) {
     result_.start = start_;
     if (limit_ < start_) {
       error(start_, "end is less than start");
       limit_ = start_;
@@ skipping 33 matching lines @@
   ModuleResult DecodeModule(WasmModule* module, bool verify_functions = true) {
     pc_ = start_;
     module->module_start = start_;
     module->module_end = limit_;
     module->min_mem_pages = 0;
     module->max_mem_pages = 0;
     module->mem_export = false;
     module->mem_external = false;
     module->origin = origin_;
 
-    bool sections[(size_t)WasmSection::Code::Max] = {false};
-
     const byte* pos = pc_;
+    int current_order = 0;
     uint32_t magic_word = consume_u32("wasm magic");
 #define BYTES(x) (x & 0xff), (x >> 8) & 0xff, (x >> 16) & 0xff, (x >> 24) & 0xff
     if (magic_word != kWasmMagic) {
       error(pos, pos,
             "expected magic word %02x %02x %02x %02x, "
             "found %02x %02x %02x %02x",
             BYTES(kWasmMagic), BYTES(magic_word));
       goto done;
     }
 
     pos = pc_;
     {
       uint32_t magic_version = consume_u32("wasm version");
       if (magic_version != kWasmVersion) {
         error(pos, pos,
               "expected version %02x %02x %02x %02x, "
               "found %02x %02x %02x %02x",
               BYTES(kWasmVersion), BYTES(magic_version));
         goto done;
       }
     }
 
     // Decode the module sections.
     while (pc_ < limit_) {
       TRACE("DecodeSection\n");
       pos = pc_;
 
-      int length;
-      uint32_t section_length = consume_u32v(&length, "section size");
-
-      int section_string_leb_length = 0;
-      uint32_t section_string_length = 0;
-      WasmSection::Code section = consume_section_name(
-          &section_string_leb_length, &section_string_length);
-      uint32_t string_and_leb_length =
-          section_string_leb_length + section_string_length;
-      if (string_and_leb_length > section_length) {
-        error(pos, pos,
-              "section string of size %u longer than total section bytes %u",
-              string_and_leb_length, section_length);
+      // Read and check the section size.
+      int section_leb_length = 0;
+      uint32_t section_length =
+          consume_u32v(&section_leb_length, "section length");
+      if (!checkAvailable(section_length)) {
+        // The section would extend beyond the end of the module.
         break;
       }
 
+      // Read the section name.
+      int string_leb_length = 0;
+      uint32_t string_length =
+          consume_u32v(&string_leb_length, "section name length");
+      if (static_cast<uint32_t>(string_leb_length) + string_length >
+          section_length) {

[JF, 2016/04/19 15:48:47]

This seems pretty impossible to cause to overflow, but it would still be good to
check all untrusted inputs.

[titzer, 2016/04/20 08:51:24]

Done. (I've refactored this a bit so the check is after the consume_bytes, which
has an overflow check inside.)

+        error(pc_, pc_,
+              "section name string %u longer than total section bytes %u",
+              string_length, section_length);
+        break;
+      }
+
+      const byte* section_name_start = pc_;
+      consume_bytes(string_length);
+      if (failed()) {
+        TRACE("Section name of length %u couldn't be read\n", string_length);
+        break;
+      }
+
+      WasmSection::Code section =
+          WasmSection::lookup(section_name_start, string_length);
+      // TODO(jfb) Linear search, it may be better to do a common-prefix search.
+      for (WasmSection::Code i = WasmSection::begin(); i != WasmSection::end();
+           i = WasmSection::next(i)) {
+        if (WasmSection::getNameLength(i) == string_length &&
+            0 == memcmp(WasmSection::getName(i), section_name_start,
+                        string_length)) {
+          section = i;
+          break;
+        }
+      }

[JF, 2016/04/19 15:48:47]

Duplicate.

[titzer, 2016/04/20 08:51:24]

Done.

+
       if (section == WasmSection::Code::Max) {
-        // Skip unknown section.
-        uint32_t skip = section_length - string_and_leb_length;
-        TRACE("skipping %u bytes from unknown section\n", skip);
-        consume_bytes(skip);
+        // Skip unknown sections.
+        TRACE("Unknown section: '");
+        for (uint32_t i = 0; i != string_length; ++i) {
+          TRACE("%c", *(section_name_start + i));
+        }
+        TRACE("'\n");
+        consume_bytes(section_length - string_length - string_leb_length);
         continue;
       }
 
-      // Each section should appear at most once.
-      CheckForPreviousSection(sections, section, false);
-      sections[(size_t)section] = true;
+      current_order = CheckSectionOrder(current_order, section);
 
       switch (section) {
         case WasmSection::Code::End:
           // Terminate section decoding.
           limit_ = pc_;
           break;
         case WasmSection::Code::Memory:
           int length;
           module->min_mem_pages = consume_u32v(&length, "min memory");
           module->max_mem_pages = consume_u32v(&length, "max memory");
           module->mem_export = consume_u8("export memory") != 0;
           break;
         case WasmSection::Code::Signatures: {
           int length;
           uint32_t signatures_count = consume_u32v(&length, "signatures count");
           module->signatures.reserve(SafeReserve(signatures_count));
           // Decode signatures.
           for (uint32_t i = 0; i < signatures_count; i++) {
             if (failed()) break;
             TRACE("DecodeSignature[%d] module+%d\n", i,
                   static_cast<int>(pc_ - start_));
             FunctionSig* s = consume_sig();  // read function sig.
             module->signatures.push_back(s);
           }
           break;
         }
         case WasmSection::Code::FunctionSignatures: {
-          // Functions require a signature table first.
-          CheckForPreviousSection(sections, WasmSection::Code::Signatures,
-                                  true);
           int length;
           uint32_t functions_count = consume_u32v(&length, "functions count");
           module->functions.reserve(SafeReserve(functions_count));
           for (uint32_t i = 0; i < functions_count; i++) {
             module->functions.push_back(
                 {nullptr, i, 0, 0, 0, 0, 0, 0, false, false});
             WasmFunction* function = &module->functions.back();
             function->sig_index = consume_sig_index(module, &function->sig);
           }
           break;
         }
         case WasmSection::Code::FunctionBodies: {
-          // Function bodies should follow signatures.
-          CheckForPreviousSection(sections,
-                                  WasmSection::Code::FunctionSignatures, true);
           int length;
           const byte* pos = pc_;
           uint32_t functions_count = consume_u32v(&length, "functions count");
           if (functions_count != module->functions.size()) {
             error(pos, pos, "function body count %u mismatch (%u expected)",
                   functions_count,
                   static_cast<uint32_t>(module->functions.size()));
             break;
           }
           for (uint32_t i = 0; i < functions_count; i++) {
             WasmFunction* function = &module->functions[i];
             int length;
             uint32_t size = consume_u32v(&length, "body size");
             function->code_start_offset = pc_offset();
             function->code_end_offset = pc_offset() + size;
 
             TRACE("  +%d  %-20s: (%d bytes)\n", pc_offset(), "function body",
                   size);
             pc_ += size;
             if (pc_ > limit_) {
               error(pc_, "function body extends beyond end of file");
             }
           }
           break;
         }
         case WasmSection::Code::Functions: {
-          // Functions require a signature table first.
-          CheckForPreviousSection(sections, WasmSection::Code::Signatures,
-                                  true);
           int length;
           uint32_t functions_count = consume_u32v(&length, "functions count");
           module->functions.reserve(SafeReserve(functions_count));
           // Set up module environment for verification.
           ModuleEnv menv;
           menv.module = module;
           menv.instance = nullptr;
           menv.origin = origin_;
           // Decode functions.
           for (uint32_t i = 0; i < functions_count; i++) {
@@ skipping 13 matching lines @@
               if (!function->external) {
                 VerifyFunctionBody(i, &menv, function);
                 if (result_.failed())
                   error(result_.error_pc, result_.error_msg.get());
               }
             }
           }
           break;
         }
         case WasmSection::Code::Names: {
-          // Names correspond to functions.
-          CheckForPreviousSection(sections,
-                                  WasmSection::Code::FunctionSignatures, true);
           int length;
           const byte* pos = pc_;
           uint32_t functions_count = consume_u32v(&length, "functions count");
           if (functions_count != module->functions.size()) {
             error(pos, pos, "function name count %u mismatch (%u expected)",
                   functions_count,
                   static_cast<uint32_t>(module->functions.size()));
             break;
           }
 
@@ skipping 75 matching lines @@
           WasmFunction* func;
           const byte* pos = pc_;
           module->start_function_index = consume_func_index(module, &func);
           if (func && func->sig->parameter_count() > 0) {
             error(pos, "invalid start function: non-zero parameter count");
             break;
           }
           break;
         }
         case WasmSection::Code::ImportTable: {
-          // Declares an import table.
-          CheckForPreviousSection(sections, WasmSection::Code::Signatures,
-                                  true);
           int length;
           uint32_t import_table_count =
               consume_u32v(&length, "import table count");
           module->import_table.reserve(SafeReserve(import_table_count));
           // Decode import table.
           for (uint32_t i = 0; i < import_table_count; i++) {
             if (failed()) break;
             TRACE("DecodeImportTable[%d] module+%d\n", i,
                   static_cast<int>(pc_ - start_));
 
@@ skipping 53 matching lines @@
     return count < kMaxReserve ? count : kMaxReserve;
   }
 
   void CheckForFunctions(WasmModule* module, WasmSection::Code section) {
     if (module->functions.size() == 0) {
       error(pc_ - 1, nullptr, "functions must appear before section %s",
             WasmSection::getName(section));
     }
   }
 
-  void CheckForPreviousSection(bool* sections, WasmSection::Code section,
-                               bool present) {
-    if (section >= WasmSection::Code::Max) return;
-    if (sections[(size_t)section] == present) return;
-    if (present) {
-      error(pc_ - 1, nullptr, "required %s section missing",
-            WasmSection::getName(section));
-    } else {
-      error(pc_ - 1, nullptr, "%s section already present",
+  int CheckSectionOrder(int current_order, WasmSection::Code section) {
+    int next_order = WasmSection::getOrder(section);
+    if (next_order == 0) return current_order;
+    if (next_order == current_order) {
+      error(pc_, pc_, "section \"%s\" already defined",
             WasmSection::getName(section));
     }
+    if (next_order < current_order) {
+      error(pc_, pc_, "section \"%s\" out of order",
+            WasmSection::getName(section));
+    }
+    return next_order;
   }
 
   // Decodes a single anonymous function starting at {start_}.
   FunctionResult DecodeSingleFunction(ModuleEnv* module_env,
                                       WasmFunction* function) {
     pc_ = start_;
     function->sig = consume_sig();            // read signature
     function->name_offset = 0;                // ---- name
     function->name_length = 0;                // ---- name length
     function->code_start_offset = off(pc_);   // ---- code start
@@ skipping 195 matching lines @@
     if (func_index >= module->functions.size()) {
       error(pos, pos, "function index %u out of bounds (%d functions)",
             func_index, static_cast<int>(module->functions.size()));
       *func = nullptr;
       return 0;
     }
     *func = &module->functions[func_index];
     return func_index;
   }
 
-  // Reads a section name.
-  WasmSection::Code consume_section_name(int* string_leb_length,
-                                         uint32_t* string_length) {
-    *string_length = consume_u32v(string_leb_length, "name length");
-    const byte* start = pc_;
-    consume_bytes(*string_length);
-    if (failed()) {
-      TRACE("Section name of length %u couldn't be read\n", *string_length);
-      return WasmSection::Code::Max;
-    }
-    // TODO(jfb) Linear search, it may be better to do a common-prefix search.
-    for (WasmSection::Code i = WasmSection::begin(); i != WasmSection::end();
-         i = WasmSection::next(i)) {
-      if (WasmSection::getNameLength(i) == *string_length &&
-          0 == memcmp(WasmSection::getName(i), start, *string_length)) {
-        return i;
-      }
-    }
-    TRACE("Unknown section: '");
-    for (uint32_t i = 0; i != *string_length; ++i) TRACE("%c", *(start + i));
-    TRACE("'\n");
-    return WasmSection::Code::Max;
-  }
-
   // Reads a single 8-bit integer, interpreting it as a local type.
   LocalType consume_local_type() {
     byte val = consume_u8("local type");
     LocalTypeCode t = static_cast<LocalTypeCode>(val);
     switch (t) {
       case kLocalVoid:
         return kAstStmt;
       case kLocalI32:
         return kAstI32;
       case kLocalI64:
@@ skipping 124 matching lines @@
     return FunctionError("size > maximum function size");
   isolate->counters()->wasm_function_size_bytes()->AddSample(
       static_cast<int>(size));
   WasmFunction* function = new WasmFunction();
   ModuleDecoder decoder(zone, function_start, function_end, kWasmOrigin);
   return decoder.DecodeSingleFunction(module_env, function);
 }
 }  // namespace wasm
 }  // namespace internal
 }  // namespace v8