Bug 182409

Summary: REGRESSION (r222824): UI process crashes in WebKit::WebBackForwardList::backItem const
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: WebKit2Assignee: David Kilzer (:ddkilzer) <ddkilzer>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bfulgham, commit-queue, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 177843    
Bug Blocks:    
Attachments:
Description Flags
Patch v1 none

Description David Kilzer (:ddkilzer) 2018-02-01 15:50:26 PST
The UI process crashes in WebKit::WebBackForwardList::backItem const during a programmatic back navigation.

In a Debug build, this assertion in WebKit::WebBackForwardList::goToItem fires instead:

        ASSERT(targetIndex != notFound);

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.JavaScriptCore      	0x0000000116506834 WTFCrash + 36 (Assertions.cpp:272)
1   com.apple.WebKit              	0x000000011137d13c WebKit::WebBackForwardList::goToItem(WebKit::WebBackForwardListItem&) + 892 (WebBackForwardList.cpp:207)
2   com.apple.WebKit              	0x000000011168788e WebKit::WebPageProxy::backForwardGoToItem(unsigned long long, WebKit::SandboxExtension::Handle&) + 254 (WebPageProxy.cpp:4547)
3   com.apple.WebKit              	0x000000011174efe1 void IPC::callMemberFunctionImpl<WebKit::WebPageProxy, void (WebKit::WebPageProxy::*)(unsigned long long, WebKit::SandboxExtension::Handle&), std::__1::tuple<unsigned long long>, 0ul, std::__1::tuple<WebKit::SandboxExtension::Handle>, 0ul>(WebKit::WebPageProxy*, void (WebKit::WebPageProxy::*)(unsigned long long, WebKit::SandboxExtension::Handle&), std::__1::tuple<unsigned long long>&&, std::__1::tuple<WebKit::SandboxExtension::Handle>&, std::__1::integer_sequence<unsigned long, 0ul>, std::__1::integer_sequence<unsigned long, 0ul>) + 193 (HandleMessage.h:55)
[...]

The debug assertion crash is due to a typo in a for loop that causes `targetIndex` to be left at the value -1, which then causes unsigned integer overflow using checked arithmetic (resulting in the release crash):

Thread 0 Crashed ↩:: Dispatch queue: com.apple.main-thread
0   com.apple.WebKit              	0x00007fff372572cd WTF::CrashOnOverflow::crash() + 5
1   com.apple.WebKit              	0x00007fff3724ea81 WTF::CrashOnOverflow::overflowed() + 9
2   com.apple.WebKit              	0x00007fff373ed706 WebKit::WebBackForwardList::backItem() const + 60
3   com.apple.WebKit              	0x00007fff3747f3fd WebKit::WebPageProxy::didChangeBackForwardList(WebKit::WebBackForwardListItem*, WTF::Vector<WTF::Ref<WebKit::WebBackForwardListItem, WTF::DumbPtrTraits<WebKit::WebBackForwardListItem> >, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&&) + 143
4   com.apple.WebKit              	0x00007fff373ed67a WebKit::WebBackForwardList::goToItem(WebKit::WebBackForwardListItem&) + 496
5   com.apple.WebKit              	0x00007fff372239b3 WebKit::WebPageProxy::backForwardGoToItem(unsigned long long, WebKit::SandboxExtension::Handle&) + 201
6   com.apple.WebKit              	0x00007fff374a713f void IPC::handleMessage<Messages::WebPageProxy::BackForwardGoToItem, WebKit::WebPageProxy, void (WebKit::WebPageProxy::*)(unsigned long long, WebKit::SandboxExtension::Handle&)>(IPC::Decoder&, IPC::Encoder&, WebKit::WebPageProxy*, void (WebKit::WebPageProxy::*)(unsigned long long, WebKit::SandboxExtension::Handle&)) + 90
[...]
Comment 1 David Kilzer (:ddkilzer) 2018-02-01 15:50:35 PST
<rdar://problem/35495094>
Comment 2 David Kilzer (:ddkilzer) 2018-02-01 15:58:29 PST
Created attachment 332920 [details]
Patch v1
Comment 3 WebKit Commit Bot 2018-02-01 16:40:36 PST
Comment on attachment 332920 [details]
Patch v1

Clearing flags on attachment: 332920

Committed r227990: <https://trac.webkit.org/changeset/227990>
Comment 4 WebKit Commit Bot 2018-02-01 16:40:38 PST
All reviewed patches have been landed.  Closing bug.