Bug 222452 – JSC Crash in makeString() while creating Error object.

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

Created attachment 421597 [details]
the test case causing a crash

Hello, following test case can cause a crash in the latest jsc.

bar_693 = '2.3023e-320';
foo_508 = bar_693.padEnd(2147483620, 1);
var newInstance = new foo_508(1, 2);

crash output:
Aborted (core dumped)

The JSC(WebKit-2.30.5) is compiled with static, debug option. 

We do some simple analysis for this crash. 'foo_508' is a large string. When 'foo_508' is used as a contructor, a TypeError should be thrown. However, during creating error message, 'foo_508' is evaluated in DerivedSources/ForwardingHeaders/wtf/text/StringConcatenate.h:makeString(). Since the foo_508's length is overflowed, a null string is returned and assigned to 'result' in makeString(), and causes the crash. I'm not sure this crash is a bug or a deliberate design.

ISec Lab.
2021.2.26

[Alexey Proskuryakov]

I don't think that we would call it entirely correct, but it is a long standing intentional behavior.


template<typename... StringTypes>
String makeString(StringTypes... strings)
{
    String result = tryMakeString(strings...);
    if (!result)
        CRASH();
    return result;
}

[Darin Adler]

The crash in makeString is 100% intentional.

The use of makeString rather than tryMakeString allows callers to select whether to crash or handle memory exhaustion differently. The full name of makeString is "makeString and crash if there is insufficient memory".

So the real question is about what invokes makeString and why.

[Alexey Proskuryakov]

Crash stack:

WTF::String WTF::makeString<WTF::String, char const*, WTF::String, char const*>(WTF::String, char const*, WTF::String, char const*) + 268

WTF::String WTF::makeString<WTF::String, char const*, WTF::String, char const*>(WTF::String, char const*, WTF::String, char const*) + 152

JSC::defaultSourceAppender(WTF::String const&, WTF::String const&, JSC::RuntimeType, JSC::ErrorInstance::SourceTextWhereErrorOccurred) + 128

JSC::ErrorInstance::finishCreation(JSC::VM&, JSC::JSGlobalObject*, WTF::String const&, WTF::String (*)(WTF::String const&, WTF::String const&, JSC::RuntimeType, JSC::ErrorInstance::SourceTextWhereErrorOccurred), JSC::RuntimeType, bool) + 1776

JSC::createTypeError(JSC::JSGlobalObject*, WTF::String const&, WTF::String (*)(WTF::String const&, WTF::String const&, JSC::RuntimeType, JSC::ErrorInstance::SourceTextWhereErrorOccurred), JSC::RuntimeType) + 192

JSC::createError(JSC::JSGlobalObject*, JSC::JSValue, WTF::String const&, WTF::String (*)(WTF::String const&, WTF::String const&, JSC::RuntimeType, JSC::ErrorInstance::SourceTextWhereErrorOccurred)) + 336

[Darin Adler]

Need one more level of backtrace so we can see who’s passing a long string to createError.

[Darin Adler]

I think the issue is that the error creation function is trying to get a description of the object, and ends up with the content of the string.

[Radar WebKit Bug Importer]

<rdar://problem/75059605>

[Darin Adler]

My take on our "architectural approach" to this: We should not pass an arbitrary, JavaScript-code-controlled, string to createError without doing some kind of length limit first.

[Alexey Proskuryakov]

Just to be clear, this is not be blocked on me or the reporter posting more information - this readily reproduces by pasting the test into the jsc tool on macOS.

[Keith Miller]

Created attachment 422739 [details]
MaybeNeedsTestRebaselines

[Mark Lam]

Comment on attachment 422739 [details]
MaybeNeedsTestRebaselines

r=me

[Keith Miller]

Talked to Darin in Slack and we settled on a 2KB max.

[Keith Miller]

Created attachment 422758 [details]
Patch for landing

[EWS]

Committed r274181: <https://commits.webkit.org/r274181>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 422758 [details].