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

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

Summary: Null pointer dereference in WebPageProxy::urlSchemeHandlerForScheme()

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	WebKit2 (show other bugs)
Version:	WebKit Nightly Build
Hardware:	Mac macOS 10.13

Importance:	P2 Normal
Assignee:	Daniel Bates

URL:
Keywords:	InRadar

Depends on:
Blocks:

Reported:	2018-02-17 12:18 PST by Benjamin Randazzo
Modified:	2018-02-19 14:50 PST (History)
CC List:	4 users (show)

See Also:

Attachments
Xcode project test case (37.69 KB, application/zip) 2018-02-17 12:18 PST, Benjamin Randazzo	no flags	Details
Patch and unit test (3.32 KB, patch) 2018-02-19 10:02 PST, Daniel Bates	achristensen: review+	Details \| Formatted Diff \| Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Benjamin Randazzo 2018-02-17 12:18:13 PST

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

Comment 1 Daniel Bates 2018-02-19 09:59:40 PST

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

Comment 2 Daniel Bates 2018-02-19 10:02:09 PST

Created attachment 334161 [details]
Patch and unit test

Comment 3 Daniel Bates 2018-02-19 10:34:17 PST

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

Comment 4 Radar WebKit Bug Importer 2018-02-19 10:46:13 PST

<rdar://problem/37676775>

Comment 5 Ryan Haddad 2018-02-19 13:14:19 PST

(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

Comment 6 Ryan Haddad 2018-02-19 14:20:18 PST

Reverted r228696 for reason:

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

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

Comment 7 Daniel Bates 2018-02-19 14:47:48 PST

(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.

Comment 8 Daniel Bates 2018-02-19 14:50:46 PST

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