WebAssembly: update arbitrary limits to what browsers use
authorjfbastien@apple.com <jfbastien@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 27 Oct 2017 18:42:27 +0000 (18:42 +0000)
committerjfbastien@apple.com <jfbastien@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 27 Oct 2017 18:42:27 +0000 (18:42 +0000)
https://bugs.webkit.org/show_bug.cgi?id=178946
<rdar://problem/34257412>
<rdar://problem/34501154>

Reviewed by Saam Barati.

https://github.com/WebAssembly/design/issues/1138 discusses the
arbitrary function size limit, which it turns out Chrome and
Firefox didn't enforce. We didn't use it because it was
ridiculously low and actual programs ran into that limit (bummer
for Edge which just shipped it...). Now that we agree on a high
arbitrary program limit, let's update it! While I'm doing this
there are a few other spots that I polished to use Checked or
better check limits overall.

* wasm/WasmB3IRGenerator.cpp:
(JSC::Wasm::B3IRGenerator::addLocal):
* wasm/WasmFormat.cpp:
(JSC::Wasm::Segment::create):
* wasm/WasmFunctionParser.h:
(JSC::Wasm::FunctionParser<Context>::parse):
* wasm/WasmInstance.cpp:
* wasm/WasmLimits.h:
* wasm/WasmModuleParser.cpp:
(JSC::Wasm::ModuleParser::parseGlobal):
(JSC::Wasm::ModuleParser::parseCode):
(JSC::Wasm::ModuleParser::parseData):
* wasm/WasmSignature.h:
(JSC::Wasm::Signature::allocatedSize):
* wasm/WasmTable.cpp:
(JSC::Wasm::Table::Table):
* wasm/js/JSWebAssemblyTable.cpp:
(JSC::JSWebAssemblyTable::JSWebAssemblyTable):
(JSC::JSWebAssemblyTable::grow):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@224122 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp
Source/JavaScriptCore/wasm/WasmFormat.cpp
Source/JavaScriptCore/wasm/WasmFunctionParser.h
Source/JavaScriptCore/wasm/WasmInstance.cpp
Source/JavaScriptCore/wasm/WasmLimits.h
Source/JavaScriptCore/wasm/WasmModuleParser.cpp
Source/JavaScriptCore/wasm/WasmSignature.h
Source/JavaScriptCore/wasm/WasmTable.cpp
Source/JavaScriptCore/wasm/js/JSWebAssemblyTable.cpp

index 5e550ef..9932eaf 100644 (file)
@@ -1,3 +1,41 @@
+2017-10-27  JF Bastien  <jfbastien@apple.com>
+
+        WebAssembly: update arbitrary limits to what browsers use
+        https://bugs.webkit.org/show_bug.cgi?id=178946
+        <rdar://problem/34257412>
+        <rdar://problem/34501154>
+
+        Reviewed by Saam Barati.
+
+        https://github.com/WebAssembly/design/issues/1138 discusses the
+        arbitrary function size limit, which it turns out Chrome and
+        Firefox didn't enforce. We didn't use it because it was
+        ridiculously low and actual programs ran into that limit (bummer
+        for Edge which just shipped it...). Now that we agree on a high
+        arbitrary program limit, let's update it! While I'm doing this
+        there are a few other spots that I polished to use Checked or
+        better check limits overall.
+
+        * wasm/WasmB3IRGenerator.cpp:
+        (JSC::Wasm::B3IRGenerator::addLocal):
+        * wasm/WasmFormat.cpp:
+        (JSC::Wasm::Segment::create):
+        * wasm/WasmFunctionParser.h:
+        (JSC::Wasm::FunctionParser<Context>::parse):
+        * wasm/WasmInstance.cpp:
+        * wasm/WasmLimits.h:
+        * wasm/WasmModuleParser.cpp:
+        (JSC::Wasm::ModuleParser::parseGlobal):
+        (JSC::Wasm::ModuleParser::parseCode):
+        (JSC::Wasm::ModuleParser::parseData):
+        * wasm/WasmSignature.h:
+        (JSC::Wasm::Signature::allocatedSize):
+        * wasm/WasmTable.cpp:
+        (JSC::Wasm::Table::Table):
+        * wasm/js/JSWebAssemblyTable.cpp:
+        (JSC::JSWebAssemblyTable::JSWebAssemblyTable):
+        (JSC::JSWebAssemblyTable::grow):
+
 2017-10-26  Michael Saboff  <msaboff@apple.com>
 
         REGRESSION(r222601): We fail to properly backtrack into a sub pattern of a parenthesis with non-zero minimum
index 48292fc..feac154 100644 (file)
@@ -511,7 +511,10 @@ void B3IRGenerator::insertConstants()
 
 auto B3IRGenerator::addLocal(Type type, uint32_t count) -> PartialResult
 {
-    WASM_COMPILE_FAIL_IF(!m_locals.tryReserveCapacity(m_locals.size() + count), "can't allocate memory for ", m_locals.size() + count, " locals");
+    Checked<uint32_t, RecordOverflow> totalBytesChecked = count;
+    totalBytesChecked += m_locals.size();
+    uint32_t totalBytes;
+    WASM_COMPILE_FAIL_IF((totalBytesChecked.safeGet(totalBytes) == CheckedState::DidOverflow) || !m_locals.tryReserveCapacity(totalBytes), "can't allocate memory for ", totalBytes, " locals");
 
     for (uint32_t i = 0; i < count; ++i) {
         Variable* local = m_proc.addVariable(toB3Type(type));
index e06e776..fdc5c9c 100644 (file)
 #if ENABLE(WEBASSEMBLY)
 
 #include "WasmMemory.h"
+#include <wtf/CheckedArithmetic.h>
 #include <wtf/FastMalloc.h>
 
 namespace JSC { namespace Wasm {
 
 Segment* Segment::create(I32InitExpr offset, uint32_t sizeInBytes)
 {
-    auto allocated = tryFastCalloc(sizeof(Segment) + sizeInBytes, 1);
+    Checked<uint32_t, RecordOverflow> totalBytesChecked = sizeInBytes;
+    totalBytesChecked += sizeof(Segment);
+    uint32_t totalBytes;
+    if (totalBytesChecked.safeGet(totalBytes) == CheckedState::DidOverflow)
+        return nullptr;
+    auto allocated = tryFastCalloc(totalBytes, 1);
     Segment* segment;
     if (!allocated.getValue(segment))
         return nullptr;
index 2c21bad..8735ce8 100644 (file)
@@ -112,14 +112,14 @@ auto FunctionParser<Context>::parse() -> Result
 
     WASM_PARSER_FAIL_IF(!m_context.addArguments(m_signature), "can't add ", m_signature.argumentCount(), " arguments to Function");
     WASM_PARSER_FAIL_IF(!parseVarUInt32(localCount), "can't get local count");
-    WASM_PARSER_FAIL_IF(localCount == std::numeric_limits<uint32_t>::max(), "Function section's local count is too big ", localCount);
+    WASM_PARSER_FAIL_IF(localCount > maxFunctionLocals, "Function section's local count is too big ", localCount, " maximum ", maxFunctionLocals);
 
     for (uint32_t i = 0; i < localCount; ++i) {
         uint32_t numberOfLocals;
         Type typeOfLocal;
 
         WASM_PARSER_FAIL_IF(!parseVarUInt32(numberOfLocals), "can't get Function's number of locals in group ", i);
-        WASM_PARSER_FAIL_IF(numberOfLocals == std::numeric_limits<uint32_t>::max(), "Function section's ", i, "th local group count is too big ", numberOfLocals);
+        WASM_PARSER_FAIL_IF(numberOfLocals > maxFunctionLocals, "Function section's ", i, "th local group count is too big ", numberOfLocals, " maximum ", maxFunctionLocals);
         WASM_PARSER_FAIL_IF(!parseValueType(typeOfLocal), "can't get Function local's type in group ", i);
         WASM_TRY_ADD_TO_CONTEXT(addLocal(typeOfLocal, numberOfLocals));
     }
index a9685ce..50a6d50 100644 (file)
 #include "config.h"
 #include "WasmInstance.h"
 
+#if ENABLE(WEBASSEMBLY)
+
 #include "Register.h"
 #include "WasmModuleInformation.h"
-
-#if ENABLE(WEBASSEMBLY)
+#include <wtf/CheckedArithmetic.h>
 
 namespace JSC { namespace Wasm {
 
 namespace {
 size_t globalMemoryByteSize(Module& module)
 {
-    return module.moduleInformation().globals.size() * sizeof(Register);
+    return (Checked<size_t>(module.moduleInformation().globals.size()) * sizeof(Register)).unsafeGet();
 }
 }
 
index 92a6e85..c6f6cdc 100644 (file)
@@ -46,6 +46,8 @@ constexpr size_t maxDataSegments = 100000;
 
 constexpr size_t maxStringSize = 100000;
 constexpr size_t maxModuleSize = 1024 * 1024 * 1024;
+constexpr size_t maxFunctionSize = 7654321;
+constexpr size_t maxFunctionLocals = 50000;
 constexpr size_t maxFunctionParams = 1000;
 
 constexpr size_t maxTableEntries = 10000000;
index 6d44ac2..0c73d3f 100644 (file)
@@ -340,7 +340,8 @@ auto ModuleParser::parseGlobal() -> PartialResult
     uint32_t globalCount;
     WASM_PARSER_FAIL_IF(!parseVarUInt32(globalCount), "can't get Global section's count");
     WASM_PARSER_FAIL_IF(globalCount > maxGlobals, "Global section's count is too big ", globalCount, " maximum ", maxGlobals);
-    WASM_PARSER_FAIL_IF(!m_info->globals.tryReserveCapacity(globalCount + m_info->firstInternalGlobal), "can't allocate memory for ", globalCount + m_info->firstInternalGlobal, " globals");
+    size_t totalBytes = globalCount + m_info->firstInternalGlobal;
+    WASM_PARSER_FAIL_IF((static_cast<uint32_t>(totalBytes) < globalCount) || !m_info->globals.tryReserveCapacity(totalBytes), "can't allocate memory for ", totalBytes, " globals");
 
     for (uint32_t globalIndex = 0; globalIndex < globalCount; ++globalIndex) {
         Global global;
@@ -474,7 +475,7 @@ auto ModuleParser::parseCode() -> PartialResult
         WASM_PARSER_FAIL_IF(!parseVarUInt32(functionSize), "can't get ", i, "th Code function's size");
         WASM_PARSER_FAIL_IF(functionSize > length(), "Code function's size ", functionSize, " exceeds the module's size ", length());
         WASM_PARSER_FAIL_IF(functionSize > length() - m_offset, "Code function's size ", functionSize, " exceeds the module's remaining size", length() - m_offset);
-        WASM_PARSER_FAIL_IF(functionSize > std::numeric_limits<uint32_t>::max(), "Code function's size ", functionSize, " is too big");
+        WASM_PARSER_FAIL_IF(functionSize > maxFunctionSize, "Code function's size ", functionSize, " is too big");
 
         m_info->functionLocationInBinary[i].start = m_offset;
         m_info->functionLocationInBinary[i].end = m_offset + functionSize;
@@ -573,7 +574,7 @@ auto ModuleParser::parseData() -> PartialResult
         WASM_FAIL_IF_HELPER_FAILS(parseInitExpr(initOpcode, initExprBits, initExprType));
         WASM_PARSER_FAIL_IF(initExprType != I32, segmentNumber, "th Data segment's init_expr must produce an i32");
         WASM_PARSER_FAIL_IF(!parseVarUInt32(dataByteLength), "can't get ", segmentNumber, "th Data segment's data byte length");
-        WASM_PARSER_FAIL_IF(dataByteLength == std::numeric_limits<uint32_t>::max(), segmentNumber, "th Data segment's data byte length is too big ", dataByteLength);
+        WASM_PARSER_FAIL_IF(dataByteLength > maxModuleSize, segmentNumber, "th Data segment's data byte length is too big ", dataByteLength, " maximum ", maxModuleSize);
 
         Segment* segment = Segment::create(makeI32InitExpr(initOpcode, initExprBits), dataByteLength);
         WASM_PARSER_FAIL_IF(!segment, "can't allocate enough memory for ", segmentNumber, "th Data segment of size ", dataByteLength);
index aaced5d..9f49ccb 100644 (file)
@@ -31,6 +31,7 @@
 #include "WasmOps.h"
 #include <cstdint>
 #include <cstring>
+#include <wtf/CheckedArithmetic.h>
 #include <wtf/HashMap.h>
 #include <wtf/HashTraits.h>
 #include <wtf/StdLibExtras.h>
@@ -64,9 +65,9 @@ class Signature : public ThreadSafeRefCounted<Signature> {
         return i + reinterpret_cast<Type*>(reinterpret_cast<char*>(this) + sizeof(Signature));
     }
     Type* storage(SignatureArgCount i) const { return const_cast<Signature*>(this)->storage(i); }
-    static size_t allocatedSize(SignatureArgCount argCount)
+    static size_t allocatedSize(Checked<SignatureArgCount> argCount)
     {
-        return sizeof(Signature) + (s_retCount + argCount) * sizeof(Type);
+        return (sizeof(Signature) + (s_retCount + argCount) * sizeof(Type)).unsafeGet();
     }
 
 public:
index 24674e2..8eeee1f 100644 (file)
@@ -54,8 +54,8 @@ Table::Table(uint32_t initial, std::optional<uint32_t> maximum)
 
     // FIXME: It might be worth trying to pre-allocate maximum here. The spec recommends doing so.
     // But for now, we're not doing that.
-    m_functions = MallocPtr<Wasm::CallableFunction>::malloc(sizeof(Wasm::CallableFunction) * static_cast<size_t>(size()));
-    m_instances = MallocPtr<Instance*>::malloc(sizeof(Instance*) * static_cast<size_t>(size()));
+    m_functions = MallocPtr<Wasm::CallableFunction>::malloc((sizeof(Wasm::CallableFunction) * Checked<size_t>(size())).unsafeGet());
+    m_instances = MallocPtr<Instance*>::malloc((sizeof(Instance*) * Checked<size_t>(size())).unsafeGet());
     for (uint32_t i = 0; i < size(); ++i) {
         new (&m_functions.get()[i]) CallableFunction();
         ASSERT(m_functions.get()[i].signatureIndex == Wasm::Signature::invalidIndex); // We rely on this in compiled code.
index 4085aac..4de4b97 100644 (file)
@@ -30,6 +30,7 @@
 
 #include "JSCInlines.h"
 #include "JSWebAssemblyInstance.h"
+#include <wtf/CheckedArithmetic.h>
 
 namespace JSC {
 
@@ -61,7 +62,7 @@ JSWebAssemblyTable::JSWebAssemblyTable(VM& vm, Structure* structure, Ref<Wasm::T
 {
     // FIXME: It might be worth trying to pre-allocate maximum here. The spec recommends doing so.
     // But for now, we're not doing that.
-    m_jsFunctions = MallocPtr<WriteBarrier<JSObject>>::malloc(sizeof(WriteBarrier<JSObject>) * static_cast<size_t>(size()));
+    m_jsFunctions = MallocPtr<WriteBarrier<JSObject>>::malloc((sizeof(WriteBarrier<JSObject>) * Checked<size_t>(size())).unsafeGet());
     for (uint32_t i = 0; i < size(); ++i)
         new(&m_jsFunctions.get()[i]) WriteBarrier<JSObject>();
 }
@@ -100,7 +101,7 @@ bool JSWebAssemblyTable::grow(uint32_t delta)
         return false;
 
     size_t newSize = grew.value();
-    m_jsFunctions.realloc(sizeof(WriteBarrier<JSObject>) * newSize);
+    m_jsFunctions.realloc((sizeof(WriteBarrier<JSObject>) * Checked<size_t>(newSize)).unsafeGet());
 
     for (size_t i = oldSize; i < newSize; ++i)
         new (&m_jsFunctions.get()[i]) WriteBarrier<JSObject>();