Bug 238048

Summary: Fix crash in Bleacher Report due to bad JSObjectRef passed to API
Product: WebKit Reporter: Keith Miller <keith_miller>
Component: New BugsAssignee: Keith Miller <keith_miller>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cdumez, cmarcelo, ews-watchlist, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Description Keith Miller 2022-03-17 14:35:43 PDT 
Fix crash in Bleecher Report due to bad JSObjectRef passed to API
Comment 1 Keith Miller 2022-03-17 14:43:12 PDT 
Created attachment 455029 [details]
Patch
Comment 2 Keith Miller 2022-03-17 14:43:16 PDT 
<rdar://problem/88766464>
Comment 3 Keith Miller 2022-03-17 14:47:05 PDT 
Created attachment 455030 [details]
Patch
Comment 4 Yusuke Suzuki 2022-03-17 14:53:25 PDT 
Comment on attachment 455030 [details]
Patch

r=me
Comment 5 Yusuke Suzuki 2022-03-17 14:53:58 PDT 
Can you file a bug removing this and putting FIXME comment on this?
Comment 6 Mark Lam 2022-03-17 14:55:11 PDT 
Comment on attachment 455030 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=455030&action=review

> Source/JavaScriptCore/ChangeLog:11
> +        short curcuiting to the non-typed array return value, 0. While technically valid

/curcuiting/circuiting/
Comment 7 Saam Barati 2022-03-17 14:56:04 PDT 
Comment on attachment 455030 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=455030&action=review

> Source/JavaScriptCore/ChangeLog:3
> +        Fix crash in Bleecher Report due to bad JSObjectRef passed to API

in various places, "Bleecher" => "Bleacher"
Comment 8 Saam Barati 2022-03-17 14:57:07 PDT 
Comment on attachment 455030 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=455030&action=review

> Source/JavaScriptCore/API/JSTypedArray.cpp:375
> +inline static bool isBleecherReport()
> +{
> +    auto bundleID = CFBundleGetIdentifier(CFBundleGetMainBundle());
> +    return bundleID
> +        && CFEqual(bundleID, CFSTR("com.bleacherreport.TeamStream"))
> +        && !linkedOnOrAfter(SDKVersion::FirstWithoutBleecherReportQuirk);
> +}

Can we cache this result using std::once?
Comment 9 Keith Miller 2022-03-17 15:06:38 PDT 
Comment on attachment 455030 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=455030&action=review

>> Source/JavaScriptCore/API/JSTypedArray.cpp:375
>> +}
> 
> Can we cache this result using std::once?

I'm fairly sure that the fact that `shouldntCrash` is static should handle that?
Comment 10 Keith Miller 2022-03-17 15:09:36 PDT 
Created attachment 455033 [details]
Patch for landing
Comment 11 EWS 2022-03-17 16:35:25 PDT 
Committed r291448 (248571@main): <https://commits.webkit.org/248571@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 455033 [details].
Comment 12 Alexey Proskuryakov 2022-03-18 17:17:58 PDT 
Comment on attachment 455033 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=455033&action=review

> Source/JavaScriptCore/API/JSTypedArray.cpp:369
> +inline static bool isBleecherReport()

Typo: Bleacher, not Bleecher.

> Source/WTF/wtf/cocoa/RuntimeApplicationChecksCocoa.h:89
> +    FirstWithoutBleecherReportQuirk = DYLD_IOS_VERSION_16_0,

Ditto.