summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAllan Sandfeld Jensen <allan.jensen@qt.io>2018-09-19 11:50:00 +0200
committerAllan Sandfeld Jensen <allan.jensen@qt.io>2018-09-28 17:02:20 +0000
commit13704549b84a86e894d707a97cfe2ad15a86f30c (patch)
tree85f60b5f9fd7f9b4d95db09329efef3b90af82b1
parent217e9181686011a64f10271e67856a2fb54f701c (diff)
[Backport] Security issue 868592v5.9.7
M69 Mega-patch for 868592 fix This CL is a collection of cherry-picks related to crbug.com/868592 fix. Specifically, this is: - The original mega-patch crrev.com/222c9ba7c6 - creis@ follow-up fix crrev.com/27986c7c955 - kouhei@ follow-up fix crrev.com/6be8b5a07bdf The original change descriptions are captured below % Change-Id lines --- Speculative crash fix for navigator.serviceworker access during unload This should fix crash/caab6eb137e58385 This CL addresses the unhandled case in crrev.com/582126 TBR=falken@chromium.org Bug: 881126, 868592 Reviewed-on: https://chromium-review.googlesource.com/1207781 Commit-Queue: Kouhei Ueno <kouhei@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Reviewed-by: Nasko Oskov <nasko@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#589419}(cherry picked from commit 6be8b5a07bdfa95c37e2da9cace7d7d4b69b31b5) Reviewed-on: https://chromium-review.googlesource.com/1212368 Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Cr-Commit-Position: refs/branch-heads/3545@{#2} Cr-Branched-From: a2bbe9dedf867fccce6d8073dc8e9c864c662bfe-refs/heads/master@{#589377} Speculative fix for additional History DocumentLoader crashes. There is no guarantee that the DocumentLoader is always attached [1], so let's introduce null checks in StateInternal and setScrollRestoration. [1] The DocumentLoader may be detached while FrameLoader::PrepareForCommit. BUG=879477, 872672 Reviewed-on: https://chromium-review.googlesource.com/1200075 Commit-Queue: Charlie Reis <creis@chromium.org> Reviewed-by: Nate Chapin <japhet@chromium.org> Cr-Commit-Position: refs/heads/master@{#588227} Speculative fix for History::ScrollRestorationInternal null deref This is a speculative fix for crash reported on crbug.com/872672 . There is no guarantee that the DocumentLoader is always attached [1], so let's introduce a null check. [1] The DocumentLoader may be detached while FrameLoader::PrepareForCommit. Bug: 872672 Reviewed-on: https://chromium-review.googlesource.com/1171972 Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Commit-Queue: Kouhei Ueno <kouhei@chromium.org> Cr-Commit-Position: refs/heads/master@{#582509} (cherry picked from commit a2a107d712d0fb754cc03ccb36630fa3ddc90f14) NavigatorServiceWorker: Avoid instantiating if being navigated away. This CL fixes a clusterfuzz crash which fails to minimize. Bug: 872320 Reviewed-on: https://chromium-review.googlesource.com/1170160 Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org> Reviewed-by: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Commit-Queue: Kouhei Ueno <kouhei@chromium.org> Cr-Commit-Position: refs/heads/master@{#582126} (cherry picked from commit d995adf5879e71074e33983b0b038d21b3f3be43) Flush microtask queue before commit Bug: 868592 Reviewed-on: https://chromium-review.googlesource.com/1164148 Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Commit-Queue: Kouhei Ueno <kouhei@chromium.org> Cr-Commit-Position: refs/heads/master@{#581124} (cherry picked from commit 38894f66a9f1d3142746572da766de1b02f8e2ce) Prevent promise reject to be sync scheduled during DocumentLoader detach (% mod: revert fetch_manager.cc change) (cherry picked from commit c415acc1e0cd8ec75e43bcb596f7bd76321f72f8) Bug: 868592 Change-Id: I50029416f0441a9f09c538716684a01cb8af93e1 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Reviewed-on: https://chromium-review.googlesource.com/1163235 Reviewed-by: Yuki Shiino <yukishiino@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Commit-Queue: Kouhei Ueno <kouhei@chromium.org> Cr-Original-Original-Commit-Position: refs/heads/master@{#580814} Reviewed-on: https://chromium-review.googlesource.com/1184122 Cr-Original-Commit-Position: refs/branch-heads/3497@{#760} Cr-Original-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} Reviewed-on: https://chromium-review.googlesource.com/1218183 Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Charlie Reis <creis@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#938} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} Reviewed-by: Michal Klocek <michal.klocek@qt.io>
-rw-r--r--chromium/content/renderer/render_frame_impl.cc11
-rw-r--r--chromium/third_party/WebKit/Source/core/frame/History.cpp27
-rw-r--r--chromium/third_party/WebKit/Source/core/loader/FrameLoader.cpp10
-rw-r--r--chromium/third_party/WebKit/Source/modules/serviceworkers/NavigatorServiceWorker.cpp22
4 files changed, 57 insertions, 13 deletions
diff --git a/chromium/content/renderer/render_frame_impl.cc b/chromium/content/renderer/render_frame_impl.cc
index b3a06009321..5b17851bc39 100644
--- a/chromium/content/renderer/render_frame_impl.cc
+++ b/chromium/content/renderer/render_frame_impl.cc
@@ -2959,8 +2959,17 @@ blink::BlameContext* RenderFrameImpl::frameBlameContext() {
blink::WebServiceWorkerProvider*
RenderFrameImpl::createServiceWorkerProvider() {
+ // Bail-out if we are about to be navigated away.
+ // We check that DocumentLoader is attached since:
+ // - This serves as the signal since the DocumentLoader is detached in
+ // FrameLoader::PrepareForCommit().
+ // - Creating ServiceWorkerProvider in
+ // RenderFrameImpl::CreateServiceWorkerProvider() assumes that there is a
+ // DocumentLoader attached to the frame.
+ if (!frame_->dataSource())
+ return nullptr;
+
// At this point we should have non-null data source.
- DCHECK(frame_->dataSource());
if (!ChildThreadImpl::current())
return nullptr; // May be null in some tests.
ServiceWorkerNetworkProvider* provider =
diff --git a/chromium/third_party/WebKit/Source/core/frame/History.cpp b/chromium/third_party/WebKit/Source/core/frame/History.cpp
index 8117a1258a6..f075a4c178a 100644
--- a/chromium/third_party/WebKit/Source/core/frame/History.cpp
+++ b/chromium/third_party/WebKit/Source/core/frame/History.cpp
@@ -74,13 +74,15 @@ SerializedScriptValue* History::state() {
}
SerializedScriptValue* History::stateInternal() const {
- if (!frame())
- return 0;
+ LocalFrame* frame = this->frame();
+ if (!frame)
+ return nullptr;
- if (HistoryItem* historyItem = frame()->loader().currentItem())
- return historyItem->stateObject();
+ HistoryItem* history_item = frame->loader().currentItem();
+ if (!history_item)
+ return nullptr;
- return 0;
+ return history_item->stateObject();
}
void History::setScrollRestoration(const String& value) {
@@ -105,12 +107,17 @@ String History::scrollRestoration() {
}
HistoryScrollRestorationType History::scrollRestorationInternal() const {
- if (frame()) {
- if (HistoryItem* historyItem = frame()->loader().currentItem())
- return historyItem->scrollRestorationType();
- }
+ const HistoryScrollRestorationType default_type = ScrollRestorationAuto;
+
+ LocalFrame* frame = this->frame();
+ if (!frame)
+ return default_type;
+
+ HistoryItem* history_item = frame->loader().currentItem();
+ if (!history_item)
+ return default_type;
- return ScrollRestorationAuto;
+ return history_item->scrollRestorationType();
}
bool History::stateChanged() const {
diff --git a/chromium/third_party/WebKit/Source/core/loader/FrameLoader.cpp b/chromium/third_party/WebKit/Source/core/loader/FrameLoader.cpp
index 124e856f77a..b15235ee321 100644
--- a/chromium/third_party/WebKit/Source/core/loader/FrameLoader.cpp
+++ b/chromium/third_party/WebKit/Source/core/loader/FrameLoader.cpp
@@ -37,8 +37,10 @@
#include "core/loader/FrameLoader.h"
#include "bindings/core/v8/DOMWrapperWorld.h"
+#include "bindings/core/v8/Microtask.h"
#include "bindings/core/v8/ScriptController.h"
#include "bindings/core/v8/SerializedScriptValue.h"
+#include "bindings/core/v8/V8PerIsolateData.h"
#include "core/HTMLNames.h"
#include "core/dom/Document.h"
#include "core/dom/Element.h"
@@ -1312,6 +1314,14 @@ bool FrameLoader::prepareForCommit() {
if (!m_frame->client())
return false;
DCHECK_EQ(m_provisionalDocumentLoader, pdl);
+
+ // Flush microtask queue so that they all run on pre-navigation context.
+ Microtask::performCheckpoint(V8PerIsolateData::mainThreadIsolate());
+
+ // Ensure that the frame_ hasn't detached from running the microtasks.
+ if (!m_frame->client())
+ return false;
+
// No more events will be dispatched so detach the Document.
// TODO(yoav): Should we also be nullifying domWindow's document (or
// domWindow) since the doc is now detached?
diff --git a/chromium/third_party/WebKit/Source/modules/serviceworkers/NavigatorServiceWorker.cpp b/chromium/third_party/WebKit/Source/modules/serviceworkers/NavigatorServiceWorker.cpp
index 700f5a31f86..09ba9d7264e 100644
--- a/chromium/third_party/WebKit/Source/modules/serviceworkers/NavigatorServiceWorker.cpp
+++ b/chromium/third_party/WebKit/Source/modules/serviceworkers/NavigatorServiceWorker.cpp
@@ -17,9 +17,27 @@ NavigatorServiceWorker::NavigatorServiceWorker(Navigator& navigator)
: nullptr) {}
NavigatorServiceWorker* NavigatorServiceWorker::from(Document& document) {
- if (!document.frame() || !document.frame()->domWindow())
+ LocalFrame* frame = document.frame();
+ if (!frame)
return nullptr;
- Navigator& navigator = *document.frame()->domWindow()->navigator();
+
+ // TODO(kouhei): Remove below after M72, since the check is now done in
+ // RenderFrameImpl::CreateServiceWorkerProvider instead.
+ //
+ // Bail-out if we are about to be navigated away.
+ // We check that DocumentLoader is attached since:
+ // - This serves as the signal since the DocumentLoader is detached in
+ // FrameLoader::PrepareForCommit().
+ // - Creating ServiceWorkerProvider in
+ // RenderFrameImpl::CreateServiceWorkerProvider() assumes that there is a
+ // DocumentLoader attached to the frame.
+ if (!frame->loader().documentLoader())
+ return nullptr;
+
+ DOMWindow* dom_window = frame->domWindow();
+ if (!dom_window)
+ return nullptr;
+ Navigator& navigator = *dom_window->navigator();
return &from(navigator);
}