MESSAGE_CHECK BackForwardItemIdentifier on incoming IPC from the WebProcess
https://bugs.webkit.org/show_bug.cgi?id=204899
Reviewed by Ryosuke Niwa.
Source/WebCore:
* Sources.txt:
* history/BackForwardItemIdentifier.cpp: Added.
(WebCore::BackForwardItemIdentifier::isValid const):
* history/BackForwardItemIdentifier.h:
(WebCore::operator!=):
Source/WebKit:
MESSAGE_CHECK BackForwardItemIdentifier on incoming IPC from the WebProcess. This is important since we use this identifier
to look up the WebBackForwardListItem in a HashMap, and looking up a bad ID could corrupt said HashMap.
* Shared/WebBackForwardListItem.cpp:
Make sure the WebBackForwardListItem is always constructed and destroyed on the main thread, to avoid corrupting
the allItems() HashMap.
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::willGoToBackForwardListItem):
(WebKit::WebPageProxy::backForwardGoToItemShared):
* UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::updateBackForwardItem):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@253163 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index d110c29..0e1edd4 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,16 @@
+2019-12-05 Chris Dumez <cdumez@apple.com>
+
+ MESSAGE_CHECK BackForwardItemIdentifier on incoming IPC from the WebProcess
+ https://bugs.webkit.org/show_bug.cgi?id=204899
+
+ Reviewed by Ryosuke Niwa.
+
+ * Sources.txt:
+ * history/BackForwardItemIdentifier.cpp: Added.
+ (WebCore::BackForwardItemIdentifier::isValid const):
+ * history/BackForwardItemIdentifier.h:
+ (WebCore::operator!=):
+
2019-12-05 Zalan Bujtas <zalan@apple.com>
[LFC][IFC] Trim trailing letter-spacing at inline container boundary
diff --git a/Source/WebCore/Sources.txt b/Source/WebCore/Sources.txt
index 917487e..cdd31a8 100644
--- a/Source/WebCore/Sources.txt
+++ b/Source/WebCore/Sources.txt
@@ -1086,6 +1086,7 @@
history/BackForwardCache.cpp
history/BackForwardController.cpp
+history/BackForwardItemIdentifier.cpp
history/CachedFrame.cpp
history/CachedPage.cpp
history/HistoryItem.cpp
diff --git a/Source/WebCore/history/BackForwardItemIdentifier.cpp b/Source/WebCore/history/BackForwardItemIdentifier.cpp
new file mode 100644
index 0000000..c0c64ff
--- /dev/null
+++ b/Source/WebCore/history/BackForwardItemIdentifier.cpp
@@ -0,0 +1,36 @@
+/*
+ * Copyright (C) 2019 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+#include "BackForwardItemIdentifier.h"
+
+namespace WebCore {
+
+bool BackForwardItemIdentifier::isValid() const
+{
+ return HashTraits<BackForwardItemIdentifier>::emptyValue() != *this && !HashTraits<BackForwardItemIdentifier>::isDeletedValue(*this);
+}
+
+}
diff --git a/Source/WebCore/history/BackForwardItemIdentifier.h b/Source/WebCore/history/BackForwardItemIdentifier.h
index 0881688..31cfbe1 100644
--- a/Source/WebCore/history/BackForwardItemIdentifier.h
+++ b/Source/WebCore/history/BackForwardItemIdentifier.h
@@ -45,6 +45,8 @@
String string() const { return makeString(processIdentifier.toUInt64(), '-', itemIdentifier.toUInt64()); }
+ WEBCORE_EXPORT bool isValid() const;
+
#if !LOG_DISABLED
const char* logString() const;
#endif
@@ -64,6 +66,11 @@
return a.processIdentifier == b.processIdentifier && a.itemIdentifier == b.itemIdentifier;
}
+inline bool operator!=(const BackForwardItemIdentifier& a, const BackForwardItemIdentifier& b)
+{
+ return !(a == b);
+}
+
template<class Encoder>
void BackForwardItemIdentifier::encode(Encoder& encoder) const
{
diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
index acb814c..2348790 100644
--- a/Source/WebKit/ChangeLog
+++ b/Source/WebKit/ChangeLog
@@ -1,3 +1,23 @@
+2019-12-05 Chris Dumez <cdumez@apple.com>
+
+ MESSAGE_CHECK BackForwardItemIdentifier on incoming IPC from the WebProcess
+ https://bugs.webkit.org/show_bug.cgi?id=204899
+
+ Reviewed by Ryosuke Niwa.
+
+ MESSAGE_CHECK BackForwardItemIdentifier on incoming IPC from the WebProcess. This is important since we use this identifier
+ to look up the WebBackForwardListItem in a HashMap, and looking up a bad ID could corrupt said HashMap.
+
+ * Shared/WebBackForwardListItem.cpp:
+ Make sure the WebBackForwardListItem is always constructed and destroyed on the main thread, to avoid corrupting
+ the allItems() HashMap.
+
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::willGoToBackForwardListItem):
+ (WebKit::WebPageProxy::backForwardGoToItemShared):
+ * UIProcess/WebProcessProxy.cpp:
+ (WebKit::WebProcessProxy::updateBackForwardItem):
+
2019-12-05 Kate Cheney <katherine_cheney@apple.com>
[MSVC] WebResourceLoadStatisticsStore.h is reporting warning C4804: '/': unsafe use of type 'bool' in operation
diff --git a/Source/WebKit/Shared/WebBackForwardListItem.cpp b/Source/WebKit/Shared/WebBackForwardListItem.cpp
index 5788488..8feac5a 100644
--- a/Source/WebKit/Shared/WebBackForwardListItem.cpp
+++ b/Source/WebKit/Shared/WebBackForwardListItem.cpp
@@ -60,6 +60,7 @@
HashMap<BackForwardItemIdentifier, WebBackForwardListItem*>& WebBackForwardListItem::allItems()
{
+ RELEASE_ASSERT(RunLoop::isMain());
static NeverDestroyed<HashMap<BackForwardItemIdentifier, WebBackForwardListItem*>> items;
return items;
}
diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
index efc82e8..35cd3ac 100644
--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
@@ -1573,6 +1573,8 @@
void WebPageProxy::willGoToBackForwardListItem(const BackForwardItemIdentifier& itemID, bool inBackForwardCache)
{
+ MESSAGE_CHECK(m_process, itemID.isValid());
+
PageClientProtector protector(pageClient());
if (auto* item = m_backForwardList->itemForID(itemID))
@@ -5925,6 +5927,8 @@
void WebPageProxy::backForwardGoToItemShared(Ref<WebProcessProxy>&& process, const BackForwardItemIdentifier& itemID, CompletionHandler<void(SandboxExtension::Handle&&)>&& completionHandler)
{
+ MESSAGE_CHECK(process, itemID.isValid());
+
auto* item = m_backForwardList->itemForID(itemID);
if (!item)
return completionHandler({ });
diff --git a/Source/WebKit/UIProcess/WebProcessProxy.cpp b/Source/WebKit/UIProcess/WebProcessProxy.cpp
index 37dd5e9..08a39a3 100644
--- a/Source/WebKit/UIProcess/WebProcessProxy.cpp
+++ b/Source/WebKit/UIProcess/WebProcessProxy.cpp
@@ -627,6 +627,8 @@
void WebProcessProxy::updateBackForwardItem(const BackForwardListItemState& itemState)
{
+ MESSAGE_CHECK(itemState.identifier.isValid());
+
auto* item = WebBackForwardListItem::itemForID(itemState.identifier);
if (!item || !isAllowedToUpdateBackForwardItem(*item))
return;
