From ea21c1ecb284c6b3b954fcc5af99a9fbc1e91b4b Mon Sep 17 00:00:00 2001 From: "achristensen@apple.com" Date: Sat, 25 Jul 2020 03:10:18 +0000 Subject: [PATCH] Null check frame in Document::dispatchDisabledAdaptationsDidChangeForMainFrame and a few other places https://bugs.webkit.org/show_bug.cgi?id=214715 Reviewed by Geoffrey Garen. Source/WebCore: Test: security/mutation-observer-frame-detach.html * dom/Document.cpp: (WebCore::Document::didBecomeCurrentDocumentInFrame): (WebCore::Document::initContentSecurityPolicy): * loader/DocumentLoader.cpp: (WebCore::DocumentLoader::commitData): Add some null checks and early returns if the frame detaches. * loader/SubframeLoader.cpp: (WebCore::FrameLoader::SubframeLoader::loadSubframe): Balance the call to incrementLoadEventDelayCount in the early return case or this test never finishes loading. LayoutTests: * security/mutation-observer-frame-detach-expected.txt: Added. * security/mutation-observer-frame-detach.html: Added. git-svn-id: https://svn.webkit.org/repository/webkit/trunk@264880 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- LayoutTests/ChangeLog | 11 +++++++ ...utation-observer-frame-detach-expected.txt | 2 ++ .../mutation-observer-frame-detach.html | 29 +++++++++++++++++++ Source/WebCore/ChangeLog | 20 +++++++++++++ Source/WebCore/dom/Document.cpp | 18 ++++++++++-- Source/WebCore/loader/DocumentLoader.cpp | 5 +++- 6 files changed, 81 insertions(+), 4 deletions(-) create mode 100644 LayoutTests/security/mutation-observer-frame-detach-expected.txt create mode 100644 LayoutTests/security/mutation-observer-frame-detach.html diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index 6aa38abc0b43..e3fd183b87c7 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,14 @@ +2020-07-24 Alex Christensen + + Null check frame in Document::dispatchDisabledAdaptationsDidChangeForMainFrame and a few other places + https://bugs.webkit.org/show_bug.cgi?id=214715 + + + Reviewed by Geoffrey Garen. + + * security/mutation-observer-frame-detach-expected.txt: Added. + * security/mutation-observer-frame-detach.html: Added. + 2020-07-24 Alex Christensen Fix assertion when highlighting in detached frames and make loads complete diff --git a/LayoutTests/security/mutation-observer-frame-detach-expected.txt b/LayoutTests/security/mutation-observer-frame-detach-expected.txt new file mode 100644 index 000000000000..aec03005a1ea --- /dev/null +++ b/LayoutTests/security/mutation-observer-frame-detach-expected.txt @@ -0,0 +1,2 @@ +ALERT: test passed because nothing crashed + diff --git a/LayoutTests/security/mutation-observer-frame-detach.html b/LayoutTests/security/mutation-observer-frame-detach.html new file mode 100644 index 000000000000..a5f63518c8a7 --- /dev/null +++ b/LayoutTests/security/mutation-observer-frame-detach.html @@ -0,0 +1,29 @@ + + +

+ +

+ diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index 307ae6f58562..49a267eab05c 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,23 @@ +2020-07-24 Alex Christensen + + Null check frame in Document::dispatchDisabledAdaptationsDidChangeForMainFrame and a few other places + https://bugs.webkit.org/show_bug.cgi?id=214715 + + + Reviewed by Geoffrey Garen. + + Test: security/mutation-observer-frame-detach.html + + * dom/Document.cpp: + (WebCore::Document::didBecomeCurrentDocumentInFrame): + (WebCore::Document::initContentSecurityPolicy): + * loader/DocumentLoader.cpp: + (WebCore::DocumentLoader::commitData): + Add some null checks and early returns if the frame detaches. + * loader/SubframeLoader.cpp: + (WebCore::FrameLoader::SubframeLoader::loadSubframe): + Balance the call to incrementLoadEventDelayCount in the early return case or this test never finishes loading. + 2020-07-24 Alex Christensen Fix assertion when highlighting in detached frames and make loads complete diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp index 48927c36efaa..3d742e719d85 100644 --- a/Source/WebCore/dom/Document.cpp +++ b/Source/WebCore/dom/Document.cpp @@ -2386,16 +2386,24 @@ void Document::createRenderTree() void Document::didBecomeCurrentDocumentInFrame() { - // FIXME: Are there cases where the document can be dislodged from the frame during the event handling below? - // If so, then m_frame could become 0, and we need to do something about that. - m_frame->script().updateDocument(); + // Many of these functions have event handlers which can detach the frame synchronously, so we must check repeatedly in this function. + if (!m_frame) + return; + if (!hasLivingRenderTree()) createRenderTree(); + if (!m_frame) + return; dispatchDisabledAdaptationsDidChangeForMainFrame(); + if (!m_frame) + return; + updateViewportArguments(); + if (!m_frame) + return; // FIXME: Doing this only for the main frame is insufficient. // Changing a subframe can also change the wheel event handler count. @@ -2406,6 +2414,8 @@ void Document::didBecomeCurrentDocumentInFrame() // unless the documents they are replacing had wheel event handlers. if (page() && m_frame->isMainFrame()) wheelEventHandlersChanged(); + if (!m_frame) + return; // Ensure that the scheduled task state of the document matches the DOM suspension state of the frame. It can // be out of sync if the DOM suspension state changed while the document was not in the frame (possibly in the @@ -6098,6 +6108,8 @@ void Document::initSecurityContext() void Document::initContentSecurityPolicy() { + if (!m_frame) + return; auto* parentFrame = m_frame->tree().parent(); if (parentFrame) contentSecurityPolicy()->copyUpgradeInsecureRequestStateFrom(*parentFrame->document()->contentSecurityPolicy()); diff --git a/Source/WebCore/loader/DocumentLoader.cpp b/Source/WebCore/loader/DocumentLoader.cpp index 2fc21498c0b1..408a13e374be 100644 --- a/Source/WebCore/loader/DocumentLoader.cpp +++ b/Source/WebCore/loader/DocumentLoader.cpp @@ -1087,7 +1087,10 @@ void DocumentLoader::commitData(const char* bytes, size_t length) bool hasBegun = m_writer.begin(documentURL(), false); m_writer.setDocumentWasLoadedAsPartOfNavigation(); - auto& document = *m_frame->document(); + auto* documentOrNull = m_frame ? m_frame->document() : nullptr; + if (!documentOrNull) + return; + auto& document = *documentOrNull; if (SecurityPolicy::allowSubstituteDataAccessToLocal() && m_originalSubstituteDataWasValid) { // If this document was loaded with substituteData, then the document can -- 2.36.0