Bug 106160 – "ASSERTION FAILED: exprStatement" in Function constructor call

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[André Bargull]

test case:
---
Function("){});(function(", "")
---

stack trace:
---
ASSERTION FAILED: exprStatement
/home/svdi/git/webkit/Source/JavaScriptCore/runtime/CodeCache.cpp(158) : JSC::UnlinkedFunctionExecutable* JSC::CodeCache::getFunctionExecutableFromGlobalCode(JSC::JSGlobalData&, const JSC::Identifier&, const JSC::SourceCode&, JSC::ParserError&)
1   0x7ffff768ab60 /home/svdi/git/webkit/WebKitBuild/Debug/lib/libJavaScriptCore.so.1(_ZN3JSC9CodeCache35getFunctionExecutableFromGlobalCodeERNS_12JSGlobalDataERKNS_10IdentifierERKNS_10SourceCodeERNS_11ParserErrorE+0x1b2) [0x7ffff768ab60]
2   0x7ffff742a903 /home/svdi/git/webkit/WebKitBuild/Debug/lib/libJavaScriptCore.so.1(_ZN3JSC26UnlinkedFunctionExecutable14fromGlobalCodeERKNS_10IdentifierEPNS_9ExecStateEPNS_8DebuggerERKNS_10SourceCodeEPPNS_8JSObjectE+0x6b) [0x7ffff742a903]
3   0x7ffff76a26b6 /home/svdi/git/webkit/WebKitBuild/Debug/lib/libJavaScriptCore.so.1(_ZN3JSC18FunctionExecutable14fromGlobalCodeERKNS_10IdentifierEPNS_9ExecStateEPNS_8DebuggerERKNS_10SourceCodeEPPNS_8JSObjectE+0x46) [0x7ffff76a26b6]
4   0x7ffff76a66b0 /home/svdi/git/webkit/WebKitBuild/Debug/lib/libJavaScriptCore.so.1(_ZN3JSC41constructFunctionSkippingEvalEnabledCheckEPNS_9ExecStateEPNS_14JSGlobalObjectERKNS_7ArgListERKNS_10IdentifierERKN3WTF6StringERKNSA_12TextPositionE+0x3de) [0x7ffff76a66b0]
5   0x7ffff76a62d0 /home/svdi/git/webkit/WebKitBuild/Debug/lib/libJavaScriptCore.so.1(_ZN3JSC17constructFunctionEPNS_9ExecStateEPNS_14JSGlobalObjectERKNS_7ArgListERKNS_10IdentifierERKN3WTF6StringERKNSA_12TextPositionE+0x8b) [0x7ffff76a62d0]
6   0x7ffff76a67d5 /home/svdi/git/webkit/WebKitBuild/Debug/lib/libJavaScriptCore.so.1(_ZN3JSC17constructFunctionEPNS_9ExecStateEPNS_14JSGlobalObjectERKNS_7ArgListE+0x6a) [0x7ffff76a67d5]
7   0x7ffff76a6204 /home/svdi/git/webkit/WebKitBuild/Debug/lib/libJavaScriptCore.so.1(+0x82d204) [0x7ffff76a6204]
8   0x7ffff75fdaa1 /home/svdi/git/webkit/WebKitBuild/Debug/lib/libJavaScriptCore.so.1(+0x784aa1) [0x7ffff75fdaa1]
9   0x7ffff76009d7 /home/svdi/git/webkit/WebKitBuild/Debug/lib/libJavaScriptCore.so.1(_ZN3JSC5LLInt9setUpCallEPNS_9ExecStateEPNS_11InstructionENS_22CodeSpecializationKindENS_7JSValueEPNS_17LLIntCallLinkInfoE+0x6b) [0x7ffff76009d7]
10  0x7ffff7600f3f /home/svdi/git/webkit/WebKitBuild/Debug/lib/libJavaScriptCore.so.1(_ZN3JSC5LLInt11genericCallEPNS_9ExecStateEPNS_11InstructionENS_22CodeSpecializationKindE+0x10a) [0x7ffff7600f3f]
11  0x7ffff75fe00e /home/svdi/git/webkit/WebKitBuild/Debug/lib/libJavaScriptCore.so.1(+0x78500e) [0x7ffff75fe00e]
12  0x7ffff7605376 /home/svdi/git/webkit/WebKitBuild/Debug/lib/libJavaScriptCore.so.1(+0x78c376) [0x7ffff7605376]

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff768ab6a in JSC::CodeCache::getFunctionExecutableFromGlobalCode (this=0x7fffb24db010, globalData=..., name=..., source=..., error=...)
    at /home/svdi/git/webkit/Source/JavaScriptCore/runtime/CodeCache.cpp:158
158	    ASSERT(exprStatement);

---

[André Bargull]

Three more test cases

Function("", "});(function(){")
=> ASSERTION FAILED: exprStatement

Function("//", "//")
=> shouldn't throw SyntaxError, but currently does

Function("/*", "*/){")
=> should throw SyntaxError, but currently doesn't

[Oliver Hunt]

(In reply to comment #1)
> Three more test cases
> 
> Function("", "});(function(){")
> => ASSERTION FAILED: exprStatement
> 
> Function("//", "//")
> => shouldn't throw SyntaxError, but currently does
> 
> Function("/*", "*/){")
> => should throw SyntaxError, but currently doesn't

O_o

Craziness.

[Mark S. Miller]

See also https://code.google.com/p/v8/issues/detail?id=2470 and https://code.google.com/p/google-caja/issues/detail?id=1616

On platforms still suffering from this bug, SES must engage in an expensive workaround.

[Geoffrey Garen]

What's SES?

[Mark S. Miller]

(In reply to comment #4)
> What's SES?

Secure EcmaScript. The most compact accurate description is probably section 2.3 of http://static.googleusercontent.com/external_content/untrusted_dlcp/research.google.com/en/us/pubs/archive/40673.pdf
Some details at https://code.google.com/p/google-caja/wiki/SES
Implementation at https://code.google.com/p/google-caja/source/browse/trunk/src/com/google/caja/ses/ , which also runs standalone. It does not depend on the rest of Caja.

Talks explaining the point at
http://www.youtube.com/watch?v=w9hHHvhZ_HY (part 1)
http://www.youtube.com/watch?v=oBqeDYETXME (part 2)

If you watch both of these parts, you can skip about the first 10 minutes of part 2.

[Geoffrey Garen]

Who are SES's primary clients?

[Mark S. Miller]

(In reply to comment #6)
> Who are SES's primary clients?

Google Sites and Google Apps Script.

[Geoffrey Garen]

<rdar://problem/13395335>

[Mark S. Miller]

(In reply to comment #8)
> <rdar://problem/13395335>

What does "rdar" mean? Does this (or the InRadar keyword above) mean that there's a fix for this in progress? Can we expect this to be fixed soon?

[Geoffrey Garen]

It means that this bug has been copied into Apple's internal bug database, named "Radar".

[Mark S. Miller]

What is the status of this issue?

[Mark S. Miller]

Is https://bugs.webkit.org/show_bug.cgi?id=131137 a duplicate of this?

[Oliver Hunt]

Created attachment 239066 [details]
Patch

[Brent Fulgham]

I tried this test case on current WebKit and I get a syntax error:

"SyntaxError: Unexpected token ')'"

Are there still cases where JSC crashes?

Please reopen with a current failure case if you believe this is still happening with the current JavaScriptCore engine.

[Oliver Hunt]

(In reply to comment #14)
> I tried this test case on current WebKit and I get a syntax error:
> 
> "SyntaxError: Unexpected token ')'"
> 
> Are there still cases where JSC crashes?
> 
> Please reopen with a current failure case if you believe this is still
> happening with the current JavaScriptCore engine.

Has it stopped asserting now?

[Brent Fulgham]

(In reply to comment #15)
> (In reply to comment #14)
> > I tried this test case on current WebKit and I get a syntax error:
> > 
> > "SyntaxError: Unexpected token ')'"
> > 
> > Are there still cases where JSC crashes?
> > 
> > Please reopen with a current failure case if you believe this is still
> > happening with the current JavaScriptCore engine.
> 
> Has it stopped asserting now?

Running a debug build of WebKit, I see no asserts when executing this code. Just the syntax error.

[Mark S. Miller]

I still see the problem on Safari, Safari Technology Preview, and Webkit Nightly when visiting
https://rawgit.com/tvcutsem/es-lab/master/src/ses/contract.html
 
The relevant test report reads:

71) Repaired: Function constructor does not verify syntax.

The test case producing this is currently at
https://github.com/tvcutsem/es-lab/blob/master/src/ses/repairES5.js#L3244

The test code in question is

Function('/*', '*/){');

which on the Webkit Nightly console produces

function anonymous(/*) {
*/){
}

[André Bargull]

(In reply to comment #14)
> Are there still cases where JSC crashes?
> 
> Please reopen with a current failure case if you believe this is still
> happening with the current JavaScriptCore engine.


New test case:
---
Function("}}; 1 * {a:{");
---

Reports this assertion failure:
---
ASSERTION FAILED: statement
---

Stack trace:
---
#0  0x00007ffff6de7098 in WTFCrash () at ../../Source/WTF/wtf/Assertions.cpp:317
#1  0x00007ffff6af2072 in JSC::CodeCache::getFunctionExecutableFromGlobalCode (this=0x7ffff0def000, vm=..., name=..., source=..., error=...)
    at ../../Source/JavaScriptCore/runtime/CodeCache.cpp:184
#2  0x00007ffff63b93e7 in JSC::UnlinkedFunctionExecutable::fromGlobalCode (name=..., exec=..., source=..., exception=@0x7fffffffc6a0: 0x0, overrideLineNumber=-1)
    at ../../Source/JavaScriptCore/bytecode/UnlinkedFunctionExecutable.cpp:178
#3  0x00007ffff6b5ccfc in JSC::FunctionExecutable::fromGlobalCode (name=..., exec=..., source=..., exception=@0x7fffffffc6a0: 0x0, overrideLineNumber=-1)
    at ../../Source/JavaScriptCore/runtime/Executable.cpp:728
#4  0x00007ffff6b60f4f in JSC::constructFunctionSkippingEvalEnabledCheck (exec=0x7fffffffcb20, globalObject=0x7fffaf1e7900, args=..., functionName=..., sourceURL=..., 
    position=..., overrideLineNumber=-1, functionConstructionMode=JSC::FunctionConstructionMode::Function, newTarget=...)
    at ../../Source/JavaScriptCore/runtime/FunctionConstructor.cpp:121
#5  0x00007ffff6b609fa in JSC::constructFunction (exec=0x7fffffffcb20, globalObject=0x7fffaf1e7900, args=..., functionName=..., sourceURL=..., position=..., 
    functionConstructionMode=JSC::FunctionConstructionMode::Function, newTarget=...) at ../../Source/JavaScriptCore/runtime/FunctionConstructor.cpp:86
#6  0x00007ffff6b611ea in JSC::constructFunction (exec=0x7fffffffcb20, globalObject=0x7fffaf1e7900, args=..., 
    functionConstructionMode=JSC::FunctionConstructionMode::Function, newTarget=...) at ../../Source/JavaScriptCore/runtime/FunctionConstructor.cpp:137
#7  0x00007ffff6b60900 in JSC::callFunctionConstructor (exec=0x7fffffffcb20) at ../../Source/JavaScriptCore/runtime/FunctionConstructor.cpp:71
...
---

[Michael Catanzaro]

*** Bug 163748 has been marked as a duplicate of this bug. ***

[Caitlin Potter (:caitp)]

I've added another version of the fix. I guess if this has been opened for so long, maybe nobody really cares about this crash, but it's not too much effort to fix it. The smaller patch ought to land, whichever that is.

[Saam Barati]

(In reply to comment #20)
> I've added another version of the fix. I guess if this has been opened for
> so long, maybe nobody really cares about this crash, but it's not too much
> effort to fix it. The smaller patch ought to land, whichever that is.

I definitely care about fixing this. Can you upload your patch from the other bug here?

[Caitlin Potter (:caitp)]

(In reply to comment #21)
> (In reply to comment #20)
> > I've added another version of the fix. I guess if this has been opened for
> > so long, maybe nobody really cares about this crash, but it's not too much
> > effort to fix it. The smaller patch ought to land, whichever that is.
> 
> I definitely care about fixing this. Can you upload your patch from the
> other bug here?

The other fix has landed already (https://trac.webkit.org/changeset/207684)