Changeset 228066 in webkit

Timestamp:

Feb 4, 2018 6:19:11 PM (6 years ago)

Author:

jmarcell@apple.com

Message:

Cherry-pick r227994. rdar://problem/37145542

Location:

branches/safari-605-branch

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/wasm/Builder.js (modified) (1 diff)
• JSTests/wasm/js-api/validate.js (modified) (1 diff)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/wasm/WasmModuleParser.cpp (modified) (4 diffs)
• Source/JavaScriptCore/wasm/WasmModuleParser.h (modified) (1 diff)
• Source/JavaScriptCore/wasm/WasmSections.h (modified) (3 diffs)

branches/safari-605-branch/JSTests/ChangeLog

@@ +1 @@
+2018-02-04  Jason Marcell  <jmarcell@apple.com>
+
+        Cherry-pick r227994. rdar://problem/37145542
+
+    2018-02-01  Keith Miller  <keith_miller@apple.com>
+
+            Fix crashes due to mishandling custom sections.
+            https://bugs.webkit.org/show_bug.cgi?id=182404
+            <rdar://problem/36935863>
+
+            Reviewed by Saam Barati.
+
+            * wasm/Builder.js:
+            (export.default.Builder.prototype._registerSectionBuilders.const.section.in.WASM.description.section.switch.section.case.string_appeared_here.this.section):
+            * wasm/js-api/validate.js:
+            (assert.truthy):
+
 2018-01-31  Jason Marcell  <jmarcell@apple.com>
 

branches/safari-605-branch/JSTests/wasm/Builder.js

@@ -576 +576 @@
             case "Element":
                 this[section] = function(...args) {
-                    if (args.length !== 0)
+                    if (args.length !== 0 && this._checked)
                         throw new Error("You're doing it wrong. This element does not take arguments. You must chain the call with another Element()");
 

branches/safari-605-branch/JSTests/wasm/js-api/validate.js

@@ -30 +30 @@
     assert.truthy(WebAssembly.validate(builder.WebAssembly().get()));
 }
+
+{
+    const builder = (new Builder());
+    builder.setChecked(false);
+
+    builder.Type().End()
+        .Import().Memory("imp", "memory", {initial: 20}).End()
+        .Unknown("test").End()
+        .Import().Memory("imp", "memory", {initial: 20}).End()
+        .Function().End()
+        .Export().End()
+        .Code()
+        .End();
+
+    assert.falsy(WebAssembly.validate(builder.WebAssembly().get()));
+}

branches/safari-605-branch/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2018-02-04  Jason Marcell  <jmarcell@apple.com>
+
+        Cherry-pick r227994. rdar://problem/37145542
+
+    2018-02-01  Keith Miller  <keith_miller@apple.com>
+
+            Fix crashes due to mishandling custom sections.
+            https://bugs.webkit.org/show_bug.cgi?id=182404
+            <rdar://problem/36935863>
+
+            Reviewed by Saam Barati.
+
+            This also cleans up some of our validation code. We also
+            mistakenly, allowed unknown (different from custom sections with
+            id: 0) section ids.
+
+            * wasm/WasmModuleParser.cpp:
+            (JSC::Wasm::ModuleParser::parse):
+            * wasm/WasmModuleParser.h:
+            * wasm/WasmSections.h:
+            (JSC::Wasm::isKnownSection):
+            (JSC::Wasm::decodeSection):
+            (JSC::Wasm::validateOrder):
+            (JSC::Wasm::makeString):
+            (JSC::Wasm::isValidSection): Deleted.
+
 2018-02-04  Jason Marcell  <jmarcell@apple.com>
 

branches/safari-605-branch/Source/JavaScriptCore/wasm/WasmModuleParser.cpp

@@ -56 +56 @@
     WASM_PARSER_FAIL_IF(versionNumber != expectedVersionNumber, "unexpected version number ", versionNumber, " expected ", expectedVersionNumber);
 
-    Section previousSection = Section::Custom;
+    // This is not really a known section.
+    Section previousKnownSection = Section::Begin;
     while (m_offset < length()) {
         uint8_t sectionByte;
@@ -63 +64 @@
 
         Section section = Section::Custom;
-        if (sectionByte) {
-            if (isValidSection(sectionByte))
-                section = static_cast<Section>(sectionByte);
-        }
+        WASM_PARSER_FAIL_IF(!decodeSection(sectionByte, section));
+        ASSERT(section != Section::Begin);
 
         uint32_t sectionLength;
-        WASM_PARSER_FAIL_IF(!validateOrder(previousSection, section), "invalid section order, ", previousSection, " followed by ", section);
+        WASM_PARSER_FAIL_IF(!validateOrder(previousKnownSection, section), "invalid section order, ", previousKnownSection, " followed by ", section);
         WASM_PARSER_FAIL_IF(!parseVarUInt32(sectionLength), "can't get ", section, " section's length");
         WASM_PARSER_FAIL_IF(sectionLength > length() - m_offset, section, "section of size ", sectionLength, " would overflow Module's size");
@@ -81 +80 @@
             break;                                                  \
         }
-        FOR_EACH_WASM_SECTION(WASM_SECTION_PARSE)
+        FOR_EACH_KNOWN_WASM_SECTION(WASM_SECTION_PARSE)
 #undef WASM_SECTION_PARSE
 
@@ -88 +87 @@
             break;
         }
+
+        case Section::Begin: {
+            RELEASE_ASSERT_NOT_REACHED();
+            break;
+        }
         }
 
         WASM_PARSER_FAIL_IF(end != m_offset, "parsing ended before the end of ", section, " section");
 
-        previousSection = section;
+
+        if (isKnownSection(section))
+            previousKnownSection = section;
     }
 

branches/safari-605-branch/Source/JavaScriptCore/wasm/WasmModuleParser.h

@@ -49 +49 @@
 
 #define WASM_SECTION_DECLARE_PARSER(NAME, ID, DESCRIPTION) PartialResult WARN_UNUSED_RETURN parse ## NAME();
-    FOR_EACH_WASM_SECTION(WASM_SECTION_DECLARE_PARSER)
+    FOR_EACH_KNOWN_WASM_SECTION(WASM_SECTION_DECLARE_PARSER)
 #undef WASM_SECTION_DECLARE_PARSER
 

branches/safari-605-branch/Source/JavaScriptCore/wasm/WasmSections.h

@@ -35 +35 @@
 namespace JSC { namespace Wasm {
 
-#define FOR_EACH_WASM_SECTION(macro) \
+#define FOR_EACH_KNOWN_WASM_SECTION(macro) \
     macro(Type,     1, "Function signature declarations") \
     macro(Import,   2, "Import declarations") \
@@ -49 +49 @@
 
 enum class Section : uint8_t {
+    // It's important that Begin is less than every other section number and that Custom is greater.
+    // This only works because section numbers are currently monotonically increasing.
+    // Also, Begin is not a real section but is used as a marker for validating the ordering
+    // of sections.
+    Begin = 0,
 #define DEFINE_WASM_SECTION_ENUM(NAME, ID, DESCRIPTION) NAME = ID,
-    FOR_EACH_WASM_SECTION(DEFINE_WASM_SECTION_ENUM)
+    FOR_EACH_KNOWN_WASM_SECTION(DEFINE_WASM_SECTION_ENUM)
 #undef DEFINE_WASM_SECTION_ENUM
     Custom
 };
+static_assert(static_cast<uint8_t>(Section::Begin) < static_cast<uint8_t>(Section::Type), "Begin should come before the first known section.");
 
 template<typename Int>
-static inline bool isValidSection(Int section)
+inline bool isKnownSection(Int section)
 {
     switch (section) {
 #define VALIDATE_SECTION(NAME, ID, DESCRIPTION) case static_cast<Int>(Section::NAME): return true;
-        FOR_EACH_WASM_SECTION(VALIDATE_SECTION)
+        FOR_EACH_KNOWN_WASM_SECTION(VALIDATE_SECTION)
 #undef VALIDATE_SECTION
     default:
@@ -67 +73 @@
 }
 
-static inline bool validateOrder(Section previous, Section next)
+inline bool decodeSection(uint8_t sectionByte, Section& section)
 {
-    if (previous == Section::Custom)
+    section = Section::Custom;
+    if (!sectionByte)
         return true;
-    return static_cast<uint8_t>(previous) < static_cast<uint8_t>(next);
+
+    if (!isKnownSection(sectionByte))
+        return false;
+
+    section = static_cast<Section>(sectionByte);
+    return true;
 }
 
-static inline const char* makeString(Section section)
+inline bool validateOrder(Section previousKnown, Section next)
+{
+    ASSERT(isKnownSection(previousKnown) || previousKnown == Section::Begin);
+    return static_cast<uint8_t>(previousKnown) < static_cast<uint8_t>(next);
+}
+
+inline const char* makeString(Section section)
 {
     switch (section) {
+    case Section::Begin:
+        return "Begin";
     case Section::Custom:
         return "Custom";
 #define STRINGIFY_SECTION_NAME(NAME, ID, DESCRIPTION) case Section::NAME: return #NAME;
-        FOR_EACH_WASM_SECTION(STRINGIFY_SECTION_NAME)
+        FOR_EACH_KNOWN_WASM_SECTION(STRINGIFY_SECTION_NAME)
 #undef STRINGIFY_SECTION_NAME
     }