[JSC] don't crash when arguments to `new Function()` produce unexpected AST

https://bugs.webkit.org/show_bug.cgi?id=163748

Reviewed by Mark Lam.

JSTests:

* stress/regress-163748.js: Added.
(assert):
(shouldThrowSyntaxError):
(GeneratorFunction):

Source/JavaScriptCore:

The ASSERT(statement); and ASSERT(funcDecl); lines are removed, replaced with blocks
to report a generic Parser error message. These lines are only possible to be reached
if the input string produced an unexpected AST, which previously could be used to crash
the process via ASSERT failure.

The node type assertions are left in the tree, as it should be impossible for a top-level
`{` to produce anything other than a Block node. If the node turns out not to be a Block,
it indicates that the (C++) caller of this function (E.g in FunctionConstructor.cpp), is
doing something incorrect. Similarly, it should be impossible for the `funcDecl` node to
be anything other than a function declaration given the conventions of the caller of this
function.

* runtime/CodeCache.cpp:
(JSC::CodeCache::getFunctionExecutableFromGlobalCode):

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

diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
index ed1fbb8..8789445 100644 (file)
--- a/JSTests/ChangeLog
+++ b/JSTests/ChangeLog
@@ -1,3 +1,15 @@
+2016-10-21  Caitlin Potter  <caitp@igalia.com>
+
+        [JSC] don't crash when arguments to `new Function()` produce unexpected AST
+        https://bugs.webkit.org/show_bug.cgi?id=163748
+
+        Reviewed by Mark Lam.
+
+        * stress/regress-163748.js: Added.
+        (assert):
+        (shouldThrowSyntaxError):
+        (GeneratorFunction):
+
 2016-10-20  Keith Miller  <keith_miller@apple.com>
 
         Add support for WASM calls

diff --git a/JSTests/stress/regress-163748.js b/JSTests/stress/regress-163748.js
new file mode 100644 (file)
index 0000000..807798a
--- /dev/null
+++ b/JSTests/stress/regress-163748.js
@@ -0,0 +1,24 @@
+function assert(cond, msg = "") {
+    if (!cond)
+        throw new Error(msg);
+}
+noInline(assert);
+
+function shouldThrowSyntaxError(str) {
+    var hadError = false;
+    try {
+        eval(str);
+    } catch (e) {
+        if (e instanceof SyntaxError)
+            hadError = true;
+    }
+    assert(hadError, "Did not throw syntax error");
+}
+noInline(shouldThrowSyntaxError);
+
+shouldThrowSyntaxError("var f = new Function('}{')");
+shouldThrowSyntaxError("var f = new Function('}}{{')");
+
+var GeneratorFunction = function*(){}.constructor;
+shouldThrowSyntaxError("var f = new GeneratorFunction('}{')");
+shouldThrowSyntaxError("var f = new GeneratorFunction('}}{{')");

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 4c1164f..c22f351 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,25 @@
+2016-10-21  Caitlin Potter  <caitp@igalia.com>
+
+        [JSC] don't crash when arguments to `new Function()` produce unexpected AST
+        https://bugs.webkit.org/show_bug.cgi?id=163748
+
+        Reviewed by Mark Lam.
+
+        The ASSERT(statement); and ASSERT(funcDecl); lines are removed, replaced with blocks
+        to report a generic Parser error message. These lines are only possible to be reached
+        if the input string produced an unexpected AST, which previously could be used to crash
+        the process via ASSERT failure.
+
+        The node type assertions are left in the tree, as it should be impossible for a top-level
+        `{` to produce anything other than a Block node. If the node turns out not to be a Block,
+        it indicates that the (C++) caller of this function (E.g in FunctionConstructor.cpp), is
+        doing something incorrect. Similarly, it should be impossible for the `funcDecl` node to
+        be anything other than a function declaration given the conventions of the caller of this
+        function.
+
+        * runtime/CodeCache.cpp:
+        (JSC::CodeCache::getFunctionExecutableFromGlobalCode):
+
 2016-10-20  Keith Miller  <keith_miller@apple.com>
 
         Add support for WASM calls

diff --git a/Source/JavaScriptCore/runtime/CodeCache.cpp b/Source/JavaScriptCore/runtime/CodeCache.cpp
index 73efab1..cc7a7f8 100644 (file)
--- a/Source/JavaScriptCore/runtime/CodeCache.cpp
+++ b/Source/JavaScriptCore/runtime/CodeCache.cpp
@@ -182,16 +182,20 @@ UnlinkedFunctionExecutable* CodeCache::getFunctionExecutableFromGlobalCode(VM& v
 
     // This function assumes an input string that would result in a single function declaration.
     StatementNode* statement = program->singleStatement();
-    ASSERT(statement);
-    ASSERT(statement->isBlock());
-    if (!statement || !statement->isBlock())
+    if (UNLIKELY(!statement)) {
+        JSToken token;
+        error = ParserError(ParserError::SyntaxError, ParserError::SyntaxErrorIrrecoverable, token, "Parser error", -1);
         return nullptr;
+    }
+    ASSERT(statement->isBlock());
 
     StatementNode* funcDecl = static_cast<BlockNode*>(statement)->singleStatement();
-    ASSERT(funcDecl);
-    ASSERT(funcDecl->isFuncDeclNode());
-    if (!funcDecl || !funcDecl->isFuncDeclNode())
+    if (UNLIKELY(!funcDecl)) {
+        JSToken token;
+        error = ParserError(ParserError::SyntaxError, ParserError::SyntaxErrorIrrecoverable, token, "Parser error", -1);
         return nullptr;
+    }
+    ASSERT(funcDecl->isFuncDeclNode());
 
     FunctionMetadataNode* metadata = static_cast<FuncDeclNode*>(funcDecl)->metadata();
     ASSERT(metadata);