Null check frame in Document::dispatchDisabledAdaptationsDidChangeForMainFrame and...
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 25 Jul 2020 03:10:18 +0000 (03:10 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 25 Jul 2020 03:10:18 +0000 (03:10 +0000)
https://bugs.webkit.org/show_bug.cgi?id=214715
<rdar://problem/65467702>

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
LayoutTests/security/mutation-observer-frame-detach-expected.txt [new file with mode: 0644]
LayoutTests/security/mutation-observer-frame-detach.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/loader/DocumentLoader.cpp

index 6aa38abc0b431b327a9e6537a3c589d435189727..e3fd183b87c771696de2740b81cd7b93e568add2 100644 (file)
@@ -1,3 +1,14 @@
+2020-07-24  Alex Christensen  <achristensen@webkit.org>
+
+        Null check frame in Document::dispatchDisabledAdaptationsDidChangeForMainFrame and a few other places
+        https://bugs.webkit.org/show_bug.cgi?id=214715
+        <rdar://problem/65467702>
+
+        Reviewed by Geoffrey Garen.
+
+        * security/mutation-observer-frame-detach-expected.txt: Added.
+        * security/mutation-observer-frame-detach.html: Added.
+
 2020-07-24  Alex Christensen  <achristensen@webkit.org>
 
         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 (file)
index 0000000..aec0300
--- /dev/null
@@ -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 (file)
index 0000000..a5f6351
--- /dev/null
@@ -0,0 +1,29 @@
+<script>
+    function runTest() {
+        var observer = new MutationObserver(()=>{p1.replaceWith(p2)});
+        observer.observe(select,{childList:true});
+        select[2] = option;
+        document.head.appendChild(p2);
+        var object = document.createElement("object");
+        var frame = document.createElement("frame");
+        audio.appendChild(option);
+        p1.appendChild(object);
+        object.data = "abc";
+        document.all[9].appendChild(frame);
+               if (window.testRunner) {
+                       testRunner.dumpAsText();
+                       alert("test passed because nothing crashed");
+               }
+    }
+</script>
+<body onload=runTest()>
+    <p id="p1">
+        <p id="p2">
+            <audio id="audio">
+                <select id="select">
+                    <option id="option"></option>
+                </select>
+            </audio>
+        </p>
+    </p>
+</body>
index 307ae6f58562f5c7630811d766fb657127028889..49a267eab05cd209168eb1b58312d4719587d384 100644 (file)
@@ -1,3 +1,23 @@
+2020-07-24  Alex Christensen  <achristensen@webkit.org>
+
+        Null check frame in Document::dispatchDisabledAdaptationsDidChangeForMainFrame and a few other places
+        https://bugs.webkit.org/show_bug.cgi?id=214715
+        <rdar://problem/65467702>
+
+        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  <achristensen@webkit.org>
 
         Fix assertion when highlighting in detached frames and make loads complete
index 48927c36efaa7c1982ce67f920dcaf64a3045137..3d742e719d850c72e7991ff574f2f39843b83a1d 100644 (file)
@@ -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());
index 2fc21498c0b138f76b3fa980d56fc4ee711c50a9..408a13e374bec78e67ac990b78a628110e1eb130 100644 (file)
@@ -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