Bug 238048 - Fix crash in Bleacher Report due to bad JSObjectRef passed to API
Summary: Fix crash in Bleacher Report due to bad JSObjectRef passed to API
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Keith Miller
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-03-17 14:35 PDT by Keith Miller
Modified: 2022-03-18 17:17 PDT (History)
10 users (show)

See Also:


Attachments
Patch (5.08 KB, patch)
2022-03-17 14:43 PDT, Keith Miller
no flags Details | Formatted Diff | Diff
Patch (5.00 KB, patch)
2022-03-17 14:47 PDT, Keith Miller
no flags Details | Formatted Diff | Diff
Patch for landing (5.00 KB, patch)
2022-03-17 15:09 PDT, Keith Miller
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.