Bug 182905 – Null pointer dereference in WebPageProxy::urlSchemeHandlerForScheme()

• Component: WebKit2
• Status: RESOLVED FIXED

[Benjamin Randazzo]

Created attachment 334109 [details]
Xcode project test case

In ./webkit/UIProcess/WebPageProxy.cpp, the method WebPageProxy::urlSchemeHandlerForScheme might be called with a NULL scheme reference, it will lead to a NULL pointer dereference.

This typically happens when trying to load a URL without a scheme component via WKWebView in a macOS/iOS app.
Here is an example code snippet written in Swift where webView is an already initialized WKWebView and the webView is asked to load a URL without scheme:
> let url = URL(string: "N")
> let request = URLRequest(url: url!)
> webView.load(request)

I have attached an example Xcode project of a macOS app to highlight the issue.

It will generate a crash with the following exception: KERN_INVALID_ADDRESS at 0x0000000000000010
Corresponding stack trace:
> Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
> 0   com.apple.WebKit              	0x00007fff50ecbdfc WTF::KeyValuePair<WTF::String, WTF::Ref<WebKit::WebURLSchemeHandler> >* WTF::HashTable<WTF::String, WTF::KeyValuePair<WTF::String, WTF::Ref<WebKit::WebURLSchemeHandler> >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::String, WTF::Ref<WebKit::WebURLSchemeHandler> > >, WTF::StringHash, WTF::HashMap<WTF::String, WTF::Ref<WebKit::WebURLSchemeHandler>, WTF::StringHash, WTF::HashTraits<WTF::String>, WTF::HashTraits<WTF::Ref<WebKit::WebURLSchemeHandler> > >::KeyValuePairTraits, WTF::HashTraits<WTF::String> >::lookup<WTF::HashMapTranslatorAdapter<WTF::HashMap<WTF::String, WTF::Ref<WebKit::WebURLSchemeHandler>, WTF::StringHash, WTF::HashTraits<WTF::String>, WTF::HashTraits<WTF::Ref<WebKit::WebURLSchemeHandler> > >::KeyValuePairTraits, WTF::IdentityHashTranslator<WTF::HashMap<WTF::String, WTF::Ref<WebKit::WebURLSchemeHandler>, WTF::StringHash, WTF::HashTraits<WTF::String>, WTF::HashTraits<WTF::Ref<WebKit::WebURLSchemeHandler> > >::KeyValuePairTraits, WTF::StringHash> >, WTF::String>(WTF::String const&) + 30
> 1   com.apple.WebKit              	0x00007fff50ec6220 WebKit::WebPageProxy::urlSchemeHandlerForScheme(WTF::String const&) + 16
> 2   com.apple.WebKit              	0x00007fff50d4cd25 WebKit::NavigationState::NavigationClient::decidePolicyForNavigationAction(WebKit::WebPageProxy&, API::NavigationAction&, WTF::Ref<WebKit::WebFramePolicyListenerProxy>&&, API::Object*)::$_0::operator()(bool) const + 177
> 3   com.apple.WebKit              	0x00007fff50d4a1b7 WebKit::NavigationState::NavigationClient::decidePolicyForNavigationAction(WebKit::WebPageProxy&, API::NavigationAction&, WTF::Ref<WebKit::WebFramePolicyListenerProxy>&&, API::Object*) + 533
> 4   com.apple.WebKit              	0x00007fff50ebc742 WebKit::WebPageProxy::decidePolicyForNavigationAction(unsigned long long, WebCore::SecurityOriginData const&, unsigned long long, WebKit::NavigationActionData const&, WebKit::FrameInfoData const&, unsigned long long, WebCore::ResourceRequest const&, WebCore::ResourceRequest const&, unsigned long long, WebKit::UserData const&, WTF::Ref<Messages::WebPageProxy::DecidePolicyForNavigationAction::DelayedReply>&&) + 1158
> 5   com.apple.WebKit              	0x00007fff50edde30 void IPC::callMemberFunctionImpl<WebKit::WebPageProxy, void (WebKit::WebPageProxy::*)(unsigned long long, WebCore::SecurityOriginData const&, unsigned long long, WebKit::NavigationActionData const&, WebKit::FrameInfoData const&, unsigned long long, WebCore::ResourceRequest const&, WebCore::ResourceRequest const&, unsigned long long, WebKit::UserData const&, WTF::Ref<Messages::WebPageProxy::DecidePolicyForNavigationAction::DelayedReply>&&), Messages::WebPageProxy::DecidePolicyForNavigationAction::DelayedReply, std::__1::tuple<unsigned long long, WebCore::SecurityOriginData, unsigned long long, WebKit::NavigationActionData, WebKit::FrameInfoData, unsigned long long, WebCore::ResourceRequest, WebCore::ResourceRequest, unsigned long long, WebKit::UserData>, 0ul, 1ul, 2ul, 3ul, 4ul, 5ul, 6ul, 7ul, 8ul, 9ul>(WebKit::WebPageProxy*, void (WebKit::WebPageProxy::*)(unsigned long long, WebCore::SecurityOriginData const&, unsigned long long, WebKit::NavigationActionData const&, WebKit::FrameInfoData const&, unsigned long long, WebCore::ResourceRequest const&, WebCore::ResourceRequest const&, unsigned long long, WebKit::UserData const&, WTF::Ref<Messages::WebPageProxy::DecidePolicyForNavigationAction::DelayedReply>&&), WTF::Ref<Messages::WebPageProxy::DecidePolicyForNavigationAction::DelayedReply>&&, std::__1::tuple<unsigned long long, WebCore::SecurityOriginData, unsigned long long, WebKit::NavigationActionData, WebKit::FrameInfoData, unsigned long long, WebCore::ResourceRequest, WebCore::ResourceRequest, unsigned long long, WebKit::UserData>&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul, 4ul, 5ul, 6ul, 7ul, 8ul, 9ul>) + 98
> 6   com.apple.WebKit              	0x00007fff50ed8c0b void IPC::handleMessageDelayed<Messages::WebPageProxy::DecidePolicyForNavigationAction, WebKit::WebPageProxy, void (WebKit::WebPageProxy::*)(unsigned long long, WebCore::SecurityOriginData const&, unsigned long long, WebKit::NavigationActionData const&, WebKit::FrameInfoData const&, unsigned long long, WebCore::ResourceRequest const&, WebCore::ResourceRequest const&, unsigned long long, WebKit::UserData const&, WTF::Ref<Messages::WebPageProxy::DecidePolicyForNavigationAction::DelayedReply>&&)>(IPC::Connection&, IPC::Decoder&, std::__1::unique_ptr<IPC::Encoder, std::__1::default_delete<IPC::Encoder> >&, WebKit::WebPageProxy*, void (WebKit::WebPageProxy::*)(unsigned long long, WebCore::SecurityOriginData const&, unsigned long long, WebKit::NavigationActionData const&, WebKit::FrameInfoData const&, unsigned long long, WebCore::ResourceRequest const&, WebCore::ResourceRequest const&, unsigned long long, WebKit::UserData const&, WTF::Ref<Messages::WebPageProxy::DecidePolicyForNavigationAction::DelayedReply>&&)) + 317
> 7   com.apple.WebKit              	0x00007fff50d47d69 IPC::MessageReceiverMap::dispatchSyncMessage(IPC::Connection&, IPC::Decoder&, std::__1::unique_ptr<IPC::Encoder, std::__1::default_delete<IPC::Encoder> >&) + 141
> 8   com.apple.WebKit              	0x00007fff50f22ff2 WebKit::WebProcessProxy::didReceiveSyncMessage(IPC::Connection&, IPC::Decoder&, std::__1::unique_ptr<IPC::Encoder, std::__1::default_delete<IPC::Encoder> >&) + 28
> 9   com.apple.WebKit              	0x00007fff50d122c5 IPC::Connection::dispatchSyncMessage(IPC::Decoder&) + 203
> 10  com.apple.WebKit              	0x00007fff50d0f97e IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >) + 104
> 11  com.apple.WebKit              	0x00007fff50d124e2 IPC::Connection::dispatchOneMessage() + 176
> 12  com.apple.JavaScriptCore      	0x00007fff4604cf09 WTF::RunLoop::performWork() + 169
> 13  com.apple.JavaScriptCore      	0x00007fff4604d1c2 WTF::RunLoop::performWork(void*) + 34
> 14  com.apple.CoreFoundation      	0x00007fff42543721 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
> 15  com.apple.CoreFoundation      	0x00007fff425fd0ac __CFRunLoopDoSource0 + 108
> 16  com.apple.CoreFoundation      	0x00007fff42526260 __CFRunLoopDoSources0 + 208
> 17  com.apple.CoreFoundation      	0x00007fff425256dd __CFRunLoopRun + 1293
> 18  com.apple.CoreFoundation      	0x00007fff42524f43 CFRunLoopRunSpecific + 483
> 19  com.apple.HIToolbox           	0x00007fff4183ce26 RunCurrentEventLoopInMode + 286
> 20  com.apple.HIToolbox           	0x00007fff4183cb96 ReceiveNextEventCommon + 613
> 21  com.apple.HIToolbox           	0x00007fff4183c914 _BlockUntilNextEventMatchingListInModeWithFilter + 64
> 22  com.apple.AppKit              	0x00007fff3fb07f5f _DPSNextEvent + 2085
> 23  com.apple.AppKit              	0x00007fff4029db4c -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 3044
> 24  com.apple.AppKit              	0x00007fff3fafcd6d -[NSApplication run] + 764
> 25  com.apple.AppKit              	0x00007fff3facbf1a NSApplicationMain + 804
> 26  com.Benjamin-Randazzo.WebViewLoadRequestBug	0x000000010d9c9bad main + 13 (AppDelegate.swift:12)
> 27  libdyld.dylib                 	0x00007fff69e3c115 start + 1

[Daniel Bates]

Thank you Benjamin for the bug report, providing detailed instructions and a test case!

[Daniel Bates]

Created attachment 334161 [details]
Patch and unit test

[Daniel Bates]

Committed r228696: <https://trac.webkit.org/changeset/228696>

[Radar WebKit Bug Importer]

<rdar://problem/37676775>

[Ryan Haddad]

(In reply to Daniel Bates from comment #3)
> Committed r228696: <https://trac.webkit.org/changeset/228696>
The API test added with this change is timing out on macOS bots:
https://build.webkit.org/builders/Apple%20High%20Sierra%20Release%20WK2%20%28Tests%29/builds/3017
https://build.webkit.org/builders/Apple%20Sierra%20Release%20WK2%20(Tests)/builds/7718

[Ryan Haddad]

Reverted r228696 for reason:

The API test added with this change is timing out on macOS.

Committed r228711: <https://trac.webkit.org/changeset/228711>

[Daniel Bates]

(In reply to Ryan Haddad from comment #6)
> Reverted r228696 for reason:
> 
> The API test added with this change is timing out on macOS.
> 
> Committed r228711: <https://trac.webkit.org/changeset/228711>

Oops! The test times out because we never receive a callback that the navigation completes since the default policy is to disallow the navigation for a app-like scheme. As it turns out it is not straightforward to write a test for this change without introducing test-specific SPI. I'm going to re-land just the null-pointer check.

[Daniel Bates]

Committed r228713: <https://trac.webkit.org/changeset/228713>