Crash under com.apple.WebKit.Networking at WebCore: WebCore::NetworkStorageSession::hasStorageAccess const: Exception Type: EXC_BAD_ACCESS (SIGSEGV) Exception Codes: KERN_INVALID_ADDRESS at 0x0000000000000010 Exception Note: EXC_CORPSE_NOTIFY Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.WebCore 0x00007fff412432a4 WebCore::NetworkStorageSession::hasStorageAccess(WTF::String const&, WTF::String const&, std::optional<unsigned long long>, unsigned long long) const + 484 1 com.apple.WebCore 0x00007fff41243002 WebCore::NetworkStorageSession::cookieStoragePartition(WebCore::URL const&, WebCore::URL const&, std::optional<unsigned long long>, std::optional<unsigned long long>) const + 706 2 com.apple.WebCore 0x00007fff41242d22 WebCore::NetworkStorageSession::cookieStoragePartition(WebCore::ResourceRequest const&, std::optional<unsigned long long>, std::optional<unsigned long long>) const + 130 3 com.apple.WebKit 0x00007fff41adc2b7 WebKit::NetworkDataTaskCocoa::NetworkDataTaskCocoa(WebKit::NetworkSession&, WebKit::NetworkDataTaskClient&, WebCore::ResourceRequest const&, unsigned long long, unsigned long long, WebCore::StoredCredentialsPolicy, WebCore::ContentSniffingPolicy, WebCore::ContentEncodingSniffingPolicy, bool, WebKit::PreconnectOnly, bool, std::optional<WebKit::NetworkActivityTracker>) + 1779 4 com.apple.WebKit 0x00007fff41ad8df1 WebKit::NetworkDataTaskCocoa::create(WebKit::NetworkSession&, WebKit::NetworkDataTaskClient&, WebCore::ResourceRequest const&, unsigned long long, unsigned long long, WebCore::StoredCredentialsPolicy, WebCore::ContentSniffingPolicy, WebCore::ContentEncodingSniffingPolicy, bool, WebKit::PreconnectOnly, bool, std::optional<WebKit::NetworkActivityTracker>) + 189 5 com.apple.WebKit 0x00007fff41ad8d0e WebKit::NetworkDataTask::create(WebKit::NetworkSession&, WebKit::NetworkDataTaskClient&, WebKit::NetworkLoadParameters const&) + 334 6 com.apple.WebKit 0x00007fff41adfff1 WebKit::NetworkLoad::initialize(WebKit::NetworkSession&) + 33 7 com.apple.WebKit 0x00007fff41b54e22 WebKit::PreconnectTask::PreconnectTask(WebKit::NetworkLoadParameters&&, WTF::CompletionHandler<void (WebCore::ResourceError const&)>&&) + 258 8 com.apple.WebKit 0x00007fff41acbe6f WebKit::NetworkConnectionToWebProcess::preconnectTo(unsigned long long, WebKit::NetworkResourceLoadParameters&&) + 93 9 com.apple.WebKit 0x00007fff41ad0020 void IPC::handleMessage<Messages::NetworkConnectionToWebProcess::PreconnectTo, WebKit::NetworkConnectionToWebProcess, void (WebKit::NetworkConnectionToWebProcess::*)(unsigned long long, WebKit::NetworkResourceLoadParameters&&)>(IPC::Decoder&, WebKit::NetworkConnectionToWebProcess*, void (WebKit::NetworkConnectionToWebProcess::*)(unsigned long long, WebKit::NetworkResourceLoadParameters&&)) + 408 10 com.apple.WebKit 0x00007fff41a5fd53 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >) + 119 11 com.apple.WebKit 0x00007fff41a626c1 IPC::Connection::dispatchOneMessage() + 177 12 com.apple.JavaScriptCore 0x00007fff360b26a7 WTF::RunLoop::performWork() + 231 13 com.apple.JavaScriptCore 0x00007fff360b2932 WTF::RunLoop::performWork(void*) + 34 14 com.apple.CoreFoundation 0x00007fff328406b0 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17 15 com.apple.CoreFoundation 0x00007fff328ddeb2 __CFRunLoopDoSource0 + 108 16 com.apple.CoreFoundation 0x00007fff32827718 __CFRunLoopDoSources0 + 195 17 com.apple.CoreFoundation 0x00007fff32826cad __CFRunLoopRun + 1225 18 com.apple.CoreFoundation 0x00007fff328265c7 CFRunLoopRunSpecific + 463 19 com.apple.Foundation 0x00007fff34c50d71 -[NSRunLoop(NSRunLoop) runMode:beforeDate:] + 280 20 com.apple.Foundation 0x00007fff34c50c46 -[NSRunLoop(NSRunLoop) run] + 76 21 libxpc.dylib 0x00007fff5fde50ee _xpc_objc_main + 567 22 libxpc.dylib 0x00007fff5fde3d45 xpc_main + 443
<rdar://problem/40750907>
<rdar://problem/40008877>
Created attachment 342295 [details] Patch
More details on the radar.
Looks good to me. Thanks for fixing this one!
BTW, didn't we have a stable repro case? Could we make a layout test out of it?
(In reply to John Wilander from comment #6) > BTW, didn't we have a stable repro case? Could we make a layout test out of > it? As noted on the radar, I believe the crash would no longer occurs after http://trac.webkit.org/r232198 but I still think it is worth hardening rather than relying on the call sites to not pass null. I will send you a layout test attempt I wrote. I got close to exercising this code path but I was not able to populate m_pagesGrantedStorageAccess from the layout test despite my efforts. Maybe you can help tweak the test :)
Comment on attachment 342295 [details] Patch Yes. This seems like a wise hardening step. Do we need the same protection in "NetworkStorageSession::grantStorageAccess", too?
Created attachment 342302 [details] Patch
(In reply to Brent Fulgham from comment #8) > Comment on attachment 342295 [details] > Patch > > Yes. This seems like a wise hardening step. > > Do we need the same protection in > "NetworkStorageSession::grantStorageAccess", too? Good idea.
Comment on attachment 342302 [details] Patch Attachment 342302 [details] did not pass win-ews (win): Output: http://webkit-queues.webkit.org/results/8091587 New failing tests: http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-video.html
Created attachment 342348 [details] Archive of layout-test-results from ews200 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews200 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Failure is unrelated.
ping review?
Comment on attachment 342302 [details] Patch r=me
Comment on attachment 342302 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=342302&action=review > Source/WebCore/platform/network/cf/NetworkStorageSessionCFNet.cpp:342 > void NetworkStorageSession::grantStorageAccess(const String& resourceDomain, const String& firstPartyDomain, std::optional<uint64_t> frameID, uint64_t pageID) Is there a reason to request storage access if firstPartyDomain is empty?
Comment on attachment 342302 [details] Patch Clearing flags on attachment: 342302 Committed r232720: <https://trac.webkit.org/changeset/232720>
All reviewed patches have been landed. Closing bug.