Title: [253460] trunk
Revision
253460
Author
[email protected]
Date
2019-12-12 17:53:20 -0800 (Thu, 12 Dec 2019)

Log Message

[JSC] Wasm init-expr should reject mutable globals
https://bugs.webkit.org/show_bug.cgi?id=205191

Reviewed by Mark Lam.

JSTests:

* wasm/js-api/global-error.js:
(assert.throws.new.WebAssembly.Module.bin):
(new.WebAssembly.Module): Deleted.
(assert.throws): Deleted.
(new.Number): Deleted.

Source/_javascript_Core:

For init-expr, expr must be a constant[1]. Constant expr, which is defined in Wasm spec, requires that, if the expr is GetGlobal,
global's mutability is immutable. Previously our imported globals are always immutable, so we are using ASSERT instead of checking
mutability in WasmSectionParser. But now, we have ability to import mutable globals. We should check mutability when parsing init-expr.
We do not have this check previously, which leads to spec-correctness issue that we can initialize globals/elements/data-segments
with snapshot values of mutable globals (this is safe, but this is not spec-compliant, and it is not reasonable semantics), while
such an attempt should be rejected when compiling Wasm modules.

This patch adds necessary checks.

[1]: https://webassembly.github.io/spec/core/valid/instructions.html#valid-constant

* wasm/WasmSectionParser.cpp:
(JSC::Wasm::SectionParser::parseInitExpr):

Modified Paths

Diff

Modified: trunk/JSTests/ChangeLog (253459 => 253460)


--- trunk/JSTests/ChangeLog	2019-12-13 01:49:54 UTC (rev 253459)
+++ trunk/JSTests/ChangeLog	2019-12-13 01:53:20 UTC (rev 253460)
@@ -1,3 +1,16 @@
+2019-12-12  Yusuke Suzuki  <[email protected]>
+
+        [JSC] Wasm init-expr should reject mutable globals
+        https://bugs.webkit.org/show_bug.cgi?id=205191
+
+        Reviewed by Mark Lam.
+
+        * wasm/js-api/global-error.js:
+        (assert.throws.new.WebAssembly.Module.bin):
+        (new.WebAssembly.Module): Deleted.
+        (assert.throws): Deleted.
+        (new.Number): Deleted.
+
 2019-12-12  Mark Lam  <[email protected]>
 
         Fix missing exception in JSValue::toWTFStringSlowCase().

Modified: trunk/JSTests/wasm/js-api/global-error.js (253459 => 253460)


--- trunk/JSTests/wasm/js-api/global-error.js	2019-12-13 01:49:54 UTC (rev 253459)
+++ trunk/JSTests/wasm/js-api/global-error.js	2019-12-13 01:53:20 UTC (rev 253460)
@@ -51,7 +51,7 @@
 
     const bin = builder.WebAssembly();
     bin.trim();
-    new WebAssembly.Module(bin.get());
+    assert.throws(() => new WebAssembly.Module(bin.get()), WebAssembly.CompileError, `WebAssembly.Module doesn't parse at byte 43: get_global import kind index 0 is mutable  (evaluating 'new WebAssembly.Module(bin.get())')`);
 }
 
 {

Modified: trunk/Source/_javascript_Core/ChangeLog (253459 => 253460)


--- trunk/Source/_javascript_Core/ChangeLog	2019-12-13 01:49:54 UTC (rev 253459)
+++ trunk/Source/_javascript_Core/ChangeLog	2019-12-13 01:53:20 UTC (rev 253460)
@@ -1,3 +1,24 @@
+2019-12-12  Yusuke Suzuki  <[email protected]>
+
+        [JSC] Wasm init-expr should reject mutable globals
+        https://bugs.webkit.org/show_bug.cgi?id=205191
+
+        Reviewed by Mark Lam.
+
+        For init-expr, expr must be a constant[1]. Constant expr, which is defined in Wasm spec, requires that, if the expr is GetGlobal,
+        global's mutability is immutable. Previously our imported globals are always immutable, so we are using ASSERT instead of checking
+        mutability in WasmSectionParser. But now, we have ability to import mutable globals. We should check mutability when parsing init-expr.
+        We do not have this check previously, which leads to spec-correctness issue that we can initialize globals/elements/data-segments
+        with snapshot values of mutable globals (this is safe, but this is not spec-compliant, and it is not reasonable semantics), while
+        such an attempt should be rejected when compiling Wasm modules.
+
+        This patch adds necessary checks.
+
+        [1]: https://webassembly.github.io/spec/core/valid/instructions.html#valid-constant
+
+        * wasm/WasmSectionParser.cpp:
+        (JSC::Wasm::SectionParser::parseInitExpr):
+
 2019-12-12  Mark Lam  <[email protected]>
 
         Fix missing exception in JSValue::toWTFStringSlowCase().

Modified: trunk/Source/_javascript_Core/wasm/WasmSectionParser.cpp (253459 => 253460)


--- trunk/Source/_javascript_Core/wasm/WasmSectionParser.cpp	2019-12-13 01:49:54 UTC (rev 253459)
+++ trunk/Source/_javascript_Core/wasm/WasmSectionParser.cpp	2019-12-13 01:53:20 UTC (rev 253460)
@@ -468,8 +468,8 @@
 
         WASM_PARSER_FAIL_IF(index >= m_info->globals.size(), "get_global's index ", index, " exceeds the number of globals ", m_info->globals.size());
         WASM_PARSER_FAIL_IF(index >= m_info->firstInternalGlobal, "get_global import kind index ", index, " exceeds the first internal global ", m_info->firstInternalGlobal);
+        WASM_PARSER_FAIL_IF(m_info->globals[index].mutability != GlobalInformation::Immutable, "get_global import kind index ", index, " is mutable ");
 
-        ASSERT(m_info->globals[index].mutability == GlobalInformation::Immutable);
         resultType = m_info->globals[index].type;
         bitsOrImportNumber = index;
         break;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to