Bug 185475 – REGRESSION (r231479): com.apple.WebCore crash in WebCore::DocumentLoader::stopLoadingAfterXFrameOptionsOrContentSecurityPolicyDenied()

• Component: New Bugs
• Status: RESOLVED FIXED

[Ryan Haddad]

Crashed Thread:        0  Dispatch queue: com.apple.main-thread

Exception Type:        EXC_BAD_ACCESS (SIGSEGV)
Exception Codes:       KERN_INVALID_ADDRESS at 0x00000000000000a4
Exception Note:        EXC_CORPSE_NOTIFY

Termination Signal:    Segmentation fault: 11
Termination Reason:    Namespace SIGNAL, Code 0xb
Terminating Process:   exc handler [0]

VM Regions Near 0xa4:
--> 
    __TEXT                 000000010112b000-000000010112d000 [    8K] r-x/rwx SM=COW  /Volumes/VOLUME/*/WebKit.framework/Versions/A/XPCServices/com.apple.WebKit.WebContent.xpc/Contents/MacOS/com.apple.WebKit.WebContent.Development

Application Specific Information:
CRASHING TEST: /security/XFrameOptions/x-frame-options-deny-delete-frame-in-load-event.html

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.WebCore             	0x0000000103371156 WebCore::DocumentLoader::stopLoadingAfterXFrameOptionsOrContentSecurityPolicyDenied(unsigned long, WebCore::ResourceResponse const&) + 198 (DocumentLoader.cpp:736)
1   com.apple.WebKit              	0x000000010146fa78 WebKit::WebResourceLoader::stopLoadingAfterXFrameOptionsOrContentSecurityPolicyDenied() + 262 (WebResourceLoader.cpp:188)
2   com.apple.WebKit              	0x000000010121f4cb WebKit::NetworkProcessConnection::didReceiveMessage(IPC::Connection&, IPC::Decoder&) + 681 (NetworkProcessConnection.cpp:112)
3   com.apple.WebKit              	0x0000000101181fd7 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >) + 119 (Connection.cpp:935)
4   com.apple.WebKit              	0x0000000101184b3e IPC::Connection::dispatchOneMessage() + 176 (Connection.cpp:964)
5   com.apple.JavaScriptCore      	0x0000000106acc94f WTF::RunLoop::performWork() + 447 (RunLoop.cpp:123)
6   com.apple.JavaScriptCore      	0x0000000106accb02 WTF::RunLoop::performWork(void*) + 34 (RunLoopCF.cpp:39)
7   com.apple.CoreFoundation      	0x00007fffac7013e1 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
8   com.apple.CoreFoundation      	0x00007fffac6e265c __CFRunLoopDoSources0 + 556
9   com.apple.CoreFoundation      	0x00007fffac6e1b46 __CFRunLoopRun + 934
10  com.apple.CoreFoundation      	0x00007fffac6e1544 CFRunLoopRunSpecific + 420
11  com.apple.HIToolbox           	0x00007fffabc40ebc RunCurrentEventLoopInMode + 240
12  com.apple.HIToolbox           	0x00007fffabc40cf1 ReceiveNextEventCommon + 432
13  com.apple.HIToolbox           	0x00007fffabc40b26 _BlockUntilNextEventMatchingListInModeWithFilter + 71
14  com.apple.AppKit              	0x00007fffaa1d7a54 _DPSNextEvent + 1120
15  com.apple.AppKit              	0x00007fffaa9537ee -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 2796
16  com.apple.AppKit              	0x00007fffaa1cc3db -[NSApplication run] + 926
17  com.apple.AppKit              	0x00007fffaa196e0e NSApplicationMain + 1237
18  libxpc.dylib                  	0x00007fffc25048c7 _xpc_objc_main + 775
19  libxpc.dylib                  	0x00007fffc25032e4 xpc_main + 494
20  com.apple.WebKit.WebContent   	0x000000010112c69a main + 490 (XPCServiceMain.mm:126)
21  libdyld.dylib                 	0x00007fffc22ab235 start + 1

https://build.webkit.org/results/Apple%20Sierra%20Release%20WK2%20(Tests)/r231480%20(9246)/results.html

[Ryan Haddad]

This is a flaky crash seen on the bots. It is attributed to http/tests/security/XFrameOptions/x-frame-options-deny-multiple-clients.html in the test results, but the crashlog blames http/tests/security/XFrameOptions/x-frame-options-deny-delete-frame-in-load-event.html

[Ryan Haddad]

I think this is related to https://trac.webkit.org/changeset/231479/webkit

[Radar WebKit Bug Importer]

<rdar://problem/40093853>

[Ryan Haddad]

This is seen on High Sierra and Sierra Release WK2 bots

[Daniel Bates]

This regression is specific to WebKit2 and the new code path that we now use to check X-Frame-Options in NetworkProcess following r231479 (*).

The issue is that DocumentLoader::stopLoadingAfterXFrameOptionsOrContentSecurityPolicyDenied() may cause its own destruction as a result of dispatching a DOM load event at the <iframe> because JavaScript can do anything in the onload handler including removing the <iframe>. And when a frame is removed we stop its document loader and destroy it among other cleanup operations (**). Prior to r231479 DocumentLoader::stopLoadingAfterXFrameOptionsOrContentSecurityPolicyDenied() was a private function and only called by DocumentLoader::responseReceived(). And DocumentLoader::responseReceived() takes out a ref on itself almost from the start and hence prevents itself from being destroyed should it need to invoke stopLoadingAfterXFrameOptionsOrContentSecurityPolicyDenied() and stopLoadingAfterXFrameOptionsOrContentSecurityPolicyDenied() causes the associated frame to be removed. Following r231479 stopLoadingAfterXFrameOptionsOrContentSecurityPolicyDenied() is now public and can also be invoked by WebResourceLoader. But WebResourceLoader does not take out a ref for DocumentLoader before invoking stopLoadingAfterXFrameOptionsOrContentSecurityPolicyDenied(). So, stopLoadingAfterXFrameOptionsOrContentSecurityPolicyDenied() can cause the document loader to destroy itself before reaching the end of the function by (**). One way to fix this issue is to have stopLoadingAfterXFrameOptionsOrContentSecurityPolicyDenied() take out a ref on itself from the getgo.

(*) We only use this new code path if both the experimental feature "Restricted HTTP Response Access" (RuntimeEnabledFeatures::sharedFeatures().restrictedHTTPResponseAccess()) and Setting::networkProcessCSPFrameAncestorsCheckingEnabled() are enabled. These two settings are enabled when running tests in WebKitTestRunner.

[Daniel Bates]

Created attachment 339994 [details]
Patch

[Daniel Bates]

Committed r231579: <https://trac.webkit.org/changeset/231579>