Bug 221565 - [iOS] Crash in ValidationBubble::show()
Summary: [iOS] Crash in ValidationBubble::show()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-02-08 12:17 PST by Ali Juma
Modified: 2021-02-25 07:05 PST (History)
8 users (show)

See Also:


Attachments
Patch (2.05 KB, patch)
2021-02-24 21:41 PST, Wenson Hsieh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ali Juma 2021-02-08 12:17:04 PST
Chrome for iOS is getting large number of crash reports in ValidationBubble::show(), all in iOS 14.

The crash stack is:

Thread 0  (id: 0x00000407) CRASHED [EXC_BAD_ACCESS / KERN_INVALID_ADDRESS @ 0x72657473 ]
0x00000001b173d468(libobjc.A.dylib + 0x00002468)objc_msgSend
0x00000001aa5886c0(WebCore + 0x00ba46c0)WebCore::ValidationBubble::show()
0x00000001a93f2798(WebKit + 0x003c1798)WebKit::WebPageProxy::showValidationMessage(WebCore::IntRect const&, WTF::String const&)
0x00000001a960cda4(WebKit + 0x005dbda4)WebKit::WebPageProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder&)
0x00000001a907d668(WebKit + 0x0004c668)IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::Decoder&)
0x00000001a938b580(WebKit + 0x0035a580)WebKit::WebProcessProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder&)
0x00000001a9061214(WebKit + 0x00030214)IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)
0x00000001a9060b58(WebKit + 0x0002fb58)IPC::Connection::dispatchIncomingMessages()
0x00000001a6ecfbc8(JavaScriptCore + 0x00dc6bc8)WTF::RunLoop::performWork()
0x00000001a6ed0714(JavaScriptCore + 0x00dc7714)WTF::RunLoop::performWork(void*)
0x000000019d33bbec(CoreFoundation + 0x0009abec)__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__
0x000000019d33baec(CoreFoundation + 0x0009aaec)__CFRunLoopDoSource0
0x000000019d33ae34(CoreFoundation + 0x00099e34)__CFRunLoopDoSources0
0x000000019d3353dc(CoreFoundation + 0x000943dc)__CFRunLoopRun
0x000000019d334b9c(CoreFoundation + 0x00093b9c)CFRunLoopRunSpecific
0x00000001b409d594(GraphicsServices + 0x00003594)GSEventRunModal
0x000000019fc262f0(UIKitCore + 0x00b2e2f0)-[UIApplication _run]
0x000000019fc2b870(UIKitCore + 0x00b33870)UIApplicationMain
0x0000000104baa4ec(Chrome -chrome_exe_main.mm:71)main
0x000000019d013564(libdyld.dylib + 0x00001564)start

The crash URLs seem to mostly be sign-in pages like:
https://schoolzone.epsb.ca/cf/index.cfm
https://myaccount.uscis.gov/
https://www.bigideasmath.com/BIM/login

Looking through changes that might have caused this, https://bugs.webkit.org/show_bug.cgi?id=208472 is the only thing I could find in ValidationBubble-related code that's new in iOS 14.
Comment 1 Wenson Hsieh 2021-02-08 12:30:32 PST
Do you happen to have repro steps for this crash? I tried showing the form validation bubble on these sites in both Chrome and Safari on trunk WebKit, but did not see any crashes.
Comment 2 Ali Juma 2021-02-08 13:05:18 PST
No repro steps unfortunately, just crash reports with URLs. The crashes are still present on iOS 14.4.
Comment 3 Radar WebKit Bug Importer 2021-02-15 12:18:14 PST
<rdar://problem/74360282>
Comment 4 Wenson Hsieh 2021-02-24 21:19:54 PST
So from code inspection, there doesn't seem to be a guarantee that this member on ValidationBubble:

UIViewController *m_presentingViewController;

...is guaranteed to be zero-initialized. This means we might actually end up calling `-presentViewController:animated:completion:` on some arbitrary pointer value in the case where we fall down this early return if `fallbackViewController` comes up `nil`:

```
void ValidationBubble::setAnchorRect(const IntRect& anchorRect, UIViewController *presentingViewController)
{
    if (!presentingViewController)
        presentingViewController = fallbackViewController(m_view);

    if (!presentingViewController)
        return;
```

The fix should be simply initializing that member as `nil`, or wrapping it in a `WeakObjCPtr` so that it can be safely accessed. That said, I'm not sure why this just started in iOS 14...

Maybe something prior to iOS 14 happened to ensure that that member always ended up being nil in this corner case.
Comment 5 Wenson Hsieh 2021-02-24 21:31:28 PST
> 
> Maybe something prior to iOS 14 happened to ensure that that member always
> ended up being nil in this corner case.

Aha — we never hit this prior to iOS 14 because we would've crashed at an earlier point, due to https://bugs.webkit.org/show_bug.cgi?id=214789 (which was first fixed in iOS 14).
Comment 6 Wenson Hsieh 2021-02-24 21:41:52 PST
Created attachment 421498 [details]
Patch
Comment 7 EWS 2021-02-25 07:05:15 PST
Committed r273482: <https://commits.webkit.org/r273482>

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