diff options
author | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2018-09-19 11:50:00 +0200 |
---|---|---|
committer | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2018-09-28 17:02:20 +0000 |
commit | 13704549b84a86e894d707a97cfe2ad15a86f30c (patch) | |
tree | 85f60b5f9fd7f9b4d95db09329efef3b90af82b1 | |
parent | 217e9181686011a64f10271e67856a2fb54f701c (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>
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); } |