[wasm] Catch invalid flags correctly

Cleanup decoding of flags so that invalid flags for sections other than
memory are caught correctly.

Bug: chromium:853453
Change-Id: Ia347d5f7672eee93ca3f6a743f06fba629f55cb5
Reviewed-on: https://chromium-review.googlesource.com/1104976
Commit-Queue: Deepti Gandluri <gdeepti@chromium.org>
Reviewed-by: Ben Smith <binji@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53972}

src/wasm/module-decoder.cc

@@ -479,20 +479,21 @@ class ModuleDecoderImpl : public Decoder {
           WasmIndirectFunctionTable* table = &module_->function_tables.back();
           table->imported = true;
           expect_u8("element type", kWasmAnyFunctionTypeCode);
+          uint8_t flags = validate_table_flags("element count");
           consume_resizable_limits(
               "element count", "elements", FLAG_wasm_max_table_size,
               &table->initial_size, &table->has_maximum_size,
-              FLAG_wasm_max_table_size, &table->maximum_size);
+              FLAG_wasm_max_table_size, &table->maximum_size, flags);
           break;
         }
         case kExternalMemory: {
           // ===== Imported memory =========================================
           if (!AddMemory(module_.get())) break;
+          uint8_t flags = validate_memory_flags(&module_->has_shared_memory);
           consume_resizable_limits(
               "memory", "pages", FLAG_wasm_max_mem_pages,
               &module_->initial_pages, &module_->has_maximum_pages,
-              kSpecMaxWasmMemoryPages, &module_->maximum_pages,
-              &module_->has_shared_memory);
+              kSpecMaxWasmMemoryPages, &module_->maximum_pages, flags);
           break;
         }
         case kExternalGlobal: {
@@ -553,10 +554,11 @@ class ModuleDecoderImpl : public Decoder {
       module_->function_tables.emplace_back();
       WasmIndirectFunctionTable* table = &module_->function_tables.back();
       expect_u8("table type", kWasmAnyFunctionTypeCode);
-      consume_resizable_limits("table elements", "elements",
-                               FLAG_wasm_max_table_size, &table->initial_size,
-                               &table->has_maximum_size,
-                               FLAG_wasm_max_table_size, &table->maximum_size);
+      uint8_t flags = validate_table_flags("table elements");
+      consume_resizable_limits(
+          "table elements", "elements", FLAG_wasm_max_table_size,
+          &table->initial_size, &table->has_maximum_size,
+          FLAG_wasm_max_table_size, &table->maximum_size, flags);
     }
   }
 
@@ -565,10 +567,11 @@ class ModuleDecoderImpl : public Decoder {
 
     for (uint32_t i = 0; ok() && i < memory_count; i++) {
       if (!AddMemory(module_.get())) break;
+      uint8_t flags = validate_memory_flags(&module_->has_shared_memory);
       consume_resizable_limits(
           "memory", "pages", FLAG_wasm_max_mem_pages, &module_->initial_pages,
           &module_->has_maximum_pages, kSpecMaxWasmMemoryPages,
-          &module_->maximum_pages, &module_->has_shared_memory);
+          &module_->maximum_pages, flags);
     }
   }
 
@@ -1095,33 +1098,43 @@ class ModuleDecoderImpl : public Decoder {
     return index;
   }
 
-  void consume_resizable_limits(const char* name, const char* units,
-                                uint32_t max_initial, uint32_t* initial,
-                                bool* has_max, uint32_t max_maximum,
-                                uint32_t* maximum,
-                                bool* has_shared_memory = nullptr) {
+  uint8_t validate_table_flags(const char* name) {
     uint8_t flags = consume_u8("resizable limits flags");
     const byte* pos = pc();
+    if (flags & 0xFE) {
+      errorf(pos - 1, "invalid %s limits flags", name);
+    }
+    return flags;
+  }
 
+  uint8_t validate_memory_flags(bool* has_shared_memory) {
+    uint8_t flags = consume_u8("resizable limits flags");
+    const byte* pos = pc();
+    *has_shared_memory = false;
     if (FLAG_experimental_wasm_threads) {
-      bool is_memory = (strcmp(name, "memory") == 0);
-      if (flags & 0xFC || (!is_memory && (flags & 0xFE))) {
-        errorf(pos - 1, "invalid %s limits flags", name);
-      }
-      if (flags == 3) {
+      if (flags & 0xFC) {
+        errorf(pos - 1, "invalid memory limits flags");
+      } else if (flags == 3) {
         DCHECK_NOT_NULL(has_shared_memory);
         *has_shared_memory = true;
       } else if (flags == 2) {
         errorf(pos - 1,
-               "%s limits flags should have maximum defined if shared is true",
-               name);
+               "memory limits flags should have maximum defined if shared is "
+               "true");
       }
     } else {
       if (flags & 0xFE) {
-        errorf(pos - 1, "invalid %s limits flags", name);
+        errorf(pos - 1, "invalid memory limits flags");
       }
     }
+    return flags;
+  }
 
+  void consume_resizable_limits(const char* name, const char* units,
+                                uint32_t max_initial, uint32_t* initial,
+                                bool* has_max, uint32_t max_maximum,
+                                uint32_t* maximum, uint8_t flags) {
+    const byte* pos = pc();
     *initial = consume_u32v("initial size");
     *has_max = false;
     if (*initial > max_initial) {

test/mjsunit/regress/wasm/regress-853453.js

+// Copyright 2018 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.
+
+// Flags: --experimental-wasm-threads
+
+assertThrows(() => new WebAssembly.Module(
+    new Uint8Array([
+      0x00, 0x61, 0x73, 0x6d,     // wasm magic
+      0x01, 0x00, 0x00, 0x00,     // wasm version
+      0x04,                       // section code
+      0x04,                       // section length
+      /* Section: Table */
+      0x01,                       // table count
+      0x70,                       // table type
+      0x03,                       // resizable limits flags
+      0x00])),
+    WebAssembly.CompileError);