[wasm] Verify boundaries of data segments when decoding modules.

R=ahaas@chromium.org,bradnelson@chromium.org
LOG=Y
BUG=chromium:575167

Review URL: https://codereview.chromium.org/1608743006

Cr-Commit-Position: refs/heads/master@{#33411}

src/wasm/module-decoder.cc

@@ -157,7 +157,7 @@ class ModuleDecoder : public Decoder {
                   static_cast<int>(pc_ - start_));
             module->data_segments->push_back({0, 0, 0});
             WasmDataSegment* segment = &module->data_segments->back();
-            DecodeDataSegmentInModule(segment);
+            DecodeDataSegmentInModule(module, segment);
           }
           break;
         }
@@ -345,14 +345,33 @@ class ModuleDecoder : public Decoder {
     }
   }
 
+  bool IsWithinLimit(uint32_t limit, uint32_t offset, uint32_t size) {
+    if (offset > limit) return false;
+    if ((offset + size) < offset) return false;  // overflow
+    return (offset + size) <= limit;
+  }
+
   // Decodes a single data segment entry inside a module starting at {pc_}.
-  void DecodeDataSegmentInModule(WasmDataSegment* segment) {
-    segment->dest_addr =
-        u32("destination");  // TODO(titzer): check it's within the memory size.
+  void DecodeDataSegmentInModule(WasmModule* module, WasmDataSegment* segment) {
+    segment->dest_addr = u32("destination");
     segment->source_offset = offset("source offset");
-    segment->source_size =
-        u32("source size");  // TODO(titzer): check the size is reasonable.
+    segment->source_size = u32("source size");
     segment->init = u8("init");
+
+    // Validate the data is in the module.
+    uint32_t module_limit = static_cast<uint32_t>(limit_ - start_);
+    if (!IsWithinLimit(module_limit, segment->source_offset,
+                       segment->source_size)) {
+      error(pc_ - sizeof(uint32_t), "segment out of bounds of module");
+    }
+
+    // Validate that the segment will fit into the (minimum) memory.
+    uint32_t memory_limit =
+        1 << (module ? module->min_mem_size_log2 : WasmModule::kMaxMemSize);
+    if (!IsWithinLimit(memory_limit, segment->dest_addr,
+                       segment->source_size)) {
+      error(pc_ - sizeof(uint32_t), "segment out of bounds of memory");
+    }
   }
 
   // Verifies the body (code) of a given function.

src/wasm/wasm-js.cc

@@ -37,6 +37,7 @@ struct RawBuffer {
 
 RawBuffer GetRawBufferArgument(
     ErrorThrower& thrower, const v8::FunctionCallbackInfo<v8::Value>& args) {
+  // TODO(titzer): allow typed array views.
   if (args.Length() < 1 || !args[0]->IsArrayBuffer()) {
     thrower.Error("Argument 0 must be an array buffer");
     return {nullptr, nullptr};
@@ -44,8 +45,6 @@ RawBuffer GetRawBufferArgument(
   Local<ArrayBuffer> buffer = Local<ArrayBuffer>::Cast(args[0]);
   ArrayBuffer::Contents contents = buffer->GetContents();
 
-  // TODO(titzer): allow offsets into buffers, views, etc.
-
   const byte* start = reinterpret_cast<const byte*>(contents.Data());
   const byte* end = start + contents.ByteLength();
 

src/wasm/wasm-module.cc

@@ -31,18 +31,8 @@ std::ostream& operator<<(std::ostream& os, const WasmModule& module) {
 
 
 std::ostream& operator<<(std::ostream& os, const WasmFunction& function) {
-  os << "WASM function with signature ";
+  os << "WASM function with signature " << *function.sig;
 
-  // TODO(titzer): factor out rendering of signatures.
-  if (function.sig->return_count() == 0) os << "v";
-  for (size_t i = 0; i < function.sig->return_count(); i++) {
-    os << WasmOpcodes::ShortNameOf(function.sig->GetReturn(i));
-  }
-  os << "_";
-  if (function.sig->parameter_count() == 0) os << "v";
-  for (size_t i = 0; i < function.sig->parameter_count(); i++) {
-    os << WasmOpcodes::ShortNameOf(function.sig->GetParam(i));
-  }
   os << " locals: ";
   if (function.local_int32_count)
     os << function.local_int32_count << " int32s ";

src/wasm/wasm-opcodes.cc

@@ -25,6 +25,20 @@ const char* WasmOpcodes::OpcodeName(WasmOpcode opcode) {
 }
 
 
+std::ostream& operator<<(std::ostream& os, const FunctionSig& sig) {
+  if (sig.return_count() == 0) os << "v";
+  for (size_t i = 0; i < sig.return_count(); i++) {
+    os << WasmOpcodes::ShortNameOf(sig.GetReturn(i));
+  }
+  os << "_";
+  if (sig.parameter_count() == 0) os << "v";
+  for (size_t i = 0; i < sig.parameter_count(); i++) {
+    os << WasmOpcodes::ShortNameOf(sig.GetParam(i));
+  }
+  return os;
+}
+
+
 #define DECLARE_SIG_ENUM(name, ...) kSigEnum_##name,
 
 

src/wasm/wasm-opcodes.h

@@ -66,6 +66,7 @@ struct MemoryAccess {
 };
 
 typedef Signature<LocalType> FunctionSig;
+std::ostream& operator<<(std::ostream& os, const FunctionSig& function);
 
 // Control expressions and blocks.
 #define FOREACH_CONTROL_OPCODE(V) \
@@ -398,7 +399,6 @@ class WasmOpcodes {
     }
   }
 
-  // TODO(titzer): remove this method
   static WasmOpcode LoadStoreOpcodeOf(MachineType type, bool store) {
     if (type == MachineType::Int8()) {
       return store ? kExprI32StoreMem8 : kExprI32LoadMem8S;

test/unittests/wasm/module-decoder-unittest.cc

@@ -45,6 +45,15 @@ struct LocalTypePair {
                    {kLocalF64, kAstF64}};
 
 
+// TODO(titzer): use these macros everywhere below.
+#define U32_LE(v)                                    \
+  static_cast<byte>(v), static_cast<byte>((v) >> 8), \
+      static_cast<byte>((v) >> 16), static_cast<byte>((v) >> 24)
+
+
+#define U16_LE(v) static_cast<byte>(v), static_cast<byte>((v) >> 8)
+
+
 TEST_F(WasmModuleVerifyTest, DecodeEmpty) {
   static const byte data[1]{kDeclEnd};
   {
@@ -463,10 +472,13 @@ TEST_F(WasmModuleVerifyTest, OneFunctionWithNopBody_WithLocals) {
 
 
 TEST_F(WasmModuleVerifyTest, OneGlobalOneFunctionWithNopBodyOneDataSegment) {
-  static const byte kCodeStartOffset = 2 + kDeclGlobalSize + 4 + 2 + 17;
+  static const byte kDeclMemorySize = 4;
+  static const byte kCodeStartOffset =
+      2 + kDeclMemorySize + kDeclGlobalSize + 4 + 2 + 17;
   static const byte kCodeEndOffset = kCodeStartOffset + 3;
 
   static const byte data[] = {
+      kDeclMemory, 28, 28, 1,
       // global#0 --------------------------------------------------
       kDeclGlobals, 1, 0, 0, 0, 0,  // name offset
       kMemU8,                       // memory type
@@ -531,24 +543,17 @@ TEST_F(WasmModuleVerifyTest, OneGlobalOneFunctionWithNopBodyOneDataSegment) {
 
 TEST_F(WasmModuleVerifyTest, OneDataSegment) {
   const byte data[] = {
-      kDeclDataSegments,
-      1,
-      0xaa,
-      0xbb,
-      0x09,
+      kDeclMemory, 28, 28, 1, kDeclDataSegments, 1, 0xaa, 0xbb, 0x09,
       0,  // dest addr
-      11,
-      0,
-      0,
+      11,          0,  0,
       0,  // source offset
-      3,
-      0,
-      0,
+      3,           0,  0,
       0,  // source size
       1,  // init
   };
 
   {
+    EXPECT_VERIFIES(data);
     ModuleResult result = DecodeModule(data, data + arraysize(data));
     EXPECT_TRUE(result.ok());
     EXPECT_EQ(0, result.val->globals->size());
@@ -565,7 +570,7 @@ TEST_F(WasmModuleVerifyTest, OneDataSegment) {
     if (result.val) delete result.val;
   }
 
-  for (size_t size = 1; size < arraysize(data); size++) {
+  for (size_t size = 5; size < arraysize(data); size++) {
     // Should fall off end of module bytes.
     ModuleResult result = DecodeModule(data, data + size);
     EXPECT_FALSE(result.ok());
@@ -576,32 +581,18 @@ TEST_F(WasmModuleVerifyTest, OneDataSegment) {
 
 TEST_F(WasmModuleVerifyTest, TwoDataSegments) {
   const byte data[] = {
-      kDeclDataSegments,
-      2,
-      0xee,
-      0xff,
-      0x07,
+      kDeclMemory, 28,   28,   1, kDeclDataSegments, 2, 0xee, 0xff, 0x07,
       0,  // dest addr
-      9,
-      0,
-      0,
+      9,           0,    0,
       0,  // #0: source offset
-      4,
-      0,
-      0,
+      4,           0,    0,
       0,  // source size
       0,  // init
-      0xcc,
-      0xdd,
-      0x06,
+      0xcc,        0xdd, 0x06,
       0,  // #1: dest addr
-      6,
-      0,
-      0,
+      6,           0,    0,
       0,  // source offset
-      10,
-      0,
-      0,
+      10,          0,    0,
       0,  // source size
       1,  // init
   };
@@ -629,7 +620,7 @@ TEST_F(WasmModuleVerifyTest, TwoDataSegments) {
     if (result.val) delete result.val;
   }
 
-  for (size_t size = 1; size < arraysize(data); size++) {
+  for (size_t size = 5; size < arraysize(data); size++) {
     // Should fall off end of module bytes.
     ModuleResult result = DecodeModule(data, data + size);
     EXPECT_FALSE(result.ok());
@@ -638,6 +629,71 @@ TEST_F(WasmModuleVerifyTest, TwoDataSegments) {
 }
 
 
+TEST_F(WasmModuleVerifyTest, DataSegmentWithInvalidSource) {
+  const int dest_addr = 0x100;
+  const byte mem_size_log2 = 15;
+  const int kDataSize = 19;
+
+  for (int source_offset = 0; source_offset < 5 + kDataSize; source_offset++) {
+    for (int source_size = -1; source_size < 5 + kDataSize; source_size += 3) {
+      byte data[] = {
+          kDeclMemory,
+          mem_size_log2,
+          mem_size_log2,
+          1,
+          kDeclDataSegments,
+          1,
+          U32_LE(dest_addr),
+          U32_LE(source_offset),
+          U32_LE(source_size),
+          1,  // init
+      };
+
+      STATIC_ASSERT(kDataSize == arraysize(data));
+
+      if (source_offset < kDataSize && source_size >= 0 &&
+          (source_offset + source_size) <= kDataSize) {
+        EXPECT_VERIFIES(data);
+      } else {
+        EXPECT_FAILURE(data);
+      }
+    }
+  }
+}
+
+
+TEST_F(WasmModuleVerifyTest, DataSegmentWithInvalidDest) {
+  const int source_size = 3;
+  const int source_offset = 11;
+
+  for (byte mem_size_log2 = 12; mem_size_log2 < 20; mem_size_log2++) {
+    int mem_size = 1 << mem_size_log2;
+
+    for (int dest_addr = mem_size - source_size;
+         dest_addr < mem_size + source_size; dest_addr++) {
+      byte data[] = {
+          kDeclMemory,
+          mem_size_log2,
+          mem_size_log2,
+          1,
+          kDeclDataSegments,
+          1,
+          U32_LE(dest_addr),
+          U32_LE(source_offset),
+          U32_LE(source_size),
+          1,  // init
+      };
+
+      if (dest_addr <= (mem_size - source_size)) {
+        EXPECT_VERIFIES(data);
+      } else {
+        EXPECT_FAILURE(data);
+      }
+    }
+  }
+}
+
+
 // To make below tests for indirect calls much shorter.
 #define FUNCTION(sig_index, external)                \
   kDeclFunctionImport, static_cast<byte>(sig_index), \