Diff
Modified: trunk/JSTests/ChangeLog (227993 => 227994)
--- trunk/JSTests/ChangeLog 2018-02-02 04:20:48 UTC (rev 227993)
+++ trunk/JSTests/ChangeLog 2018-02-02 04:30:37 UTC (rev 227994)
@@ -1,3 +1,16 @@
+2018-02-01 Keith Miller <[email protected]>
+
+ 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 Saam Barati <[email protected]>
JSC incorrectly interpreting script, sets Global Property instead of Global Lexical variable (LiteralParser / JSONP path)
Modified: trunk/JSTests/wasm/Builder.js (227993 => 227994)
--- trunk/JSTests/wasm/Builder.js 2018-02-02 04:20:48 UTC (rev 227993)
+++ trunk/JSTests/wasm/Builder.js 2018-02-02 04:30:37 UTC (rev 227994)
@@ -575,7 +575,7 @@
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()");
const s = this._addSection(section);
Modified: trunk/JSTests/wasm/js-api/validate.js (227993 => 227994)
--- trunk/JSTests/wasm/js-api/validate.js 2018-02-02 04:20:48 UTC (rev 227993)
+++ trunk/JSTests/wasm/js-api/validate.js 2018-02-02 04:30:37 UTC (rev 227994)
@@ -29,3 +29,19 @@
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()));
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (227993 => 227994)
--- trunk/Source/_javascript_Core/ChangeLog 2018-02-02 04:20:48 UTC (rev 227993)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-02-02 04:30:37 UTC (rev 227994)
@@ -1,3 +1,25 @@
+2018-02-01 Keith Miller <[email protected]>
+
+ 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-01 Michael Catanzaro <[email protected]>
-Wreturn-type warning in DFGObjectAllocationSinkingPhase.cpp
Modified: trunk/Source/_javascript_Core/wasm/WasmModuleParser.cpp (227993 => 227994)
--- trunk/Source/_javascript_Core/wasm/WasmModuleParser.cpp 2018-02-02 04:20:48 UTC (rev 227993)
+++ trunk/Source/_javascript_Core/wasm/WasmModuleParser.cpp 2018-02-02 04:30:37 UTC (rev 227994)
@@ -55,7 +55,8 @@
WASM_PARSER_FAIL_IF(!parseUInt32(versionNumber), "can't parse version number");
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;
@@ -62,13 +63,11 @@
WASM_PARSER_FAIL_IF(!parseUInt7(sectionByte), "can't get section byte");
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");
@@ -80,7 +79,7 @@
WASM_FAIL_IF_HELPER_FAILS(parse ## NAME()); \
break; \
}
- FOR_EACH_WASM_SECTION(WASM_SECTION_PARSE)
+ FOR_EACH_KNOWN_WASM_SECTION(WASM_SECTION_PARSE)
#undef WASM_SECTION_PARSE
case Section::Custom: {
@@ -87,11 +86,18 @@
WASM_FAIL_IF_HELPER_FAILS(parseCustom(sectionLength));
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;
}
return { };
Modified: trunk/Source/_javascript_Core/wasm/WasmModuleParser.h (227993 => 227994)
--- trunk/Source/_javascript_Core/wasm/WasmModuleParser.h 2018-02-02 04:20:48 UTC (rev 227993)
+++ trunk/Source/_javascript_Core/wasm/WasmModuleParser.h 2018-02-02 04:30:37 UTC (rev 227994)
@@ -48,7 +48,7 @@
private:
#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
PartialResult WARN_UNUSED_RETURN parseCustom(uint32_t);
Modified: trunk/Source/_javascript_Core/wasm/WasmSections.h (227993 => 227994)
--- trunk/Source/_javascript_Core/wasm/WasmSections.h 2018-02-02 04:20:48 UTC (rev 227993)
+++ trunk/Source/_javascript_Core/wasm/WasmSections.h 2018-02-02 04:30:37 UTC (rev 227994)
@@ -34,7 +34,7 @@
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") \
macro(Function, 3, "Function declarations") \
@@ -48,18 +48,24 @@
macro(Data, 11, "Data segments")
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:
return false;
@@ -66,20 +72,34 @@
}
}
-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
}
ASSERT_NOT_REACHED();
