Disallow loading webarchives as iframes
authorjiewen_tan@apple.com <jiewen_tan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 27 Nov 2018 19:38:18 +0000 (19:38 +0000)
committerjiewen_tan@apple.com <jiewen_tan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 27 Nov 2018 19:38:18 +0000 (19:38 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191728
<rdar://problem/45524528>

Reviewed by Youenn Fablet.

Source/WebCore:

Disallow loading webarchives as iframes. We don't allow loading remote webarchives.
Now, this policy is hardened to disallow loading webarchives as iframes for local
documents as well.

To allow old tests still be able to run, a flag is added to always allow loading local
webarchives in document. The flag can be set via window.internals.

Tests: webarchive/loading/test-loading-archive-subresource.html
       webarchive/loading/test-loading-top-archive.html

* dom/Document.h:
(WebCore::Document::setAlwaysAllowLocalWebarchive):
(WebCore::Document::alwaysAllowLocalWebarchive):
* loader/DocumentLoader.cpp:
(WebCore::disallowWebArchive):
(WebCore::DocumentLoader::continueAfterContentPolicy):
(WebCore::isRemoteWebArchive): Deleted.
* testing/Internals.cpp:
(WebCore::Internals::setAlwaysAllowLocalWebarchive const):
* testing/Internals.h:
* testing/Internals.idl:

Source/WebKit:

* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::unableToImplementPolicy):
Add a check to prevent null pointer dereference.

LayoutTests:

* platform/mac-wk1/webarchive/loading/test-loading-archive-subresource-expected.txt: Added.
* platform/mac/fast/loader/webarchive-encoding-respected.html:
* webarchive/loading/cache-expired-subresource.html:
* webarchive/loading/mainresource-null-mimetype-crash.html:
* webarchive/loading/missing-data.html:
* webarchive/loading/resources/test-loading-archive-main.webarchive: Copied from LayoutTests/webarchive/loading/test-loading-archive.html.
* webarchive/loading/test-loading-archive-subresource-expected.txt: Added.
* webarchive/loading/test-loading-archive-subresource-null-mimetype.html:
* webarchive/loading/test-loading-archive-subresource.html: Copied from LayoutTests/webarchive/loading/test-loading-archive.html.
* webarchive/loading/test-loading-archive.html:
* webarchive/loading/test-loading-top-archive-expected.txt: Added.
* webarchive/loading/test-loading-top-archive.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@238562 268f45cc-cd09-0410-ab3c-d52691b4dbfc

22 files changed:
LayoutTests/ChangeLog
LayoutTests/platform/mac-wk1/webarchive/loading/test-loading-archive-subresource-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac/fast/loader/webarchive-encoding-respected.html
LayoutTests/webarchive/loading/cache-expired-subresource.html
LayoutTests/webarchive/loading/mainresource-null-mimetype-crash.html
LayoutTests/webarchive/loading/missing-data.html
LayoutTests/webarchive/loading/resources/top.webarchive [new file with mode: 0644]
LayoutTests/webarchive/loading/test-loading-archive-subresource-expected.txt [new file with mode: 0644]
LayoutTests/webarchive/loading/test-loading-archive-subresource-null-mimetype.html
LayoutTests/webarchive/loading/test-loading-archive-subresource.html [new file with mode: 0644]
LayoutTests/webarchive/loading/test-loading-archive.html
LayoutTests/webarchive/loading/test-loading-top-archive-expected.txt [new file with mode: 0644]
LayoutTests/webarchive/loading/test-loading-top-archive.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.h
Source/WebCore/loader/DocumentLoader.cpp
Source/WebCore/loader/DocumentLoader.h
Source/WebCore/testing/Internals.cpp
Source/WebCore/testing/Internals.h
Source/WebCore/testing/Internals.idl
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/WebPageProxy.cpp

index ad1e8bfa19be0e2172bf52498d29762a206088ff..a97491c75f23f6be138416f31d730d29696c9d9e 100644 (file)
@@ -1,3 +1,24 @@
+2018-11-16  Jiewen Tan  <jiewen_tan@apple.com>
+
+        Disallow loading webarchives as iframes
+        https://bugs.webkit.org/show_bug.cgi?id=191728
+        <rdar://problem/45524528>
+
+        Reviewed by Youenn Fablet.
+
+        * platform/mac-wk1/webarchive/loading/test-loading-archive-subresource-expected.txt: Added.
+        * platform/mac/fast/loader/webarchive-encoding-respected.html:
+        * webarchive/loading/cache-expired-subresource.html:
+        * webarchive/loading/mainresource-null-mimetype-crash.html:
+        * webarchive/loading/missing-data.html:
+        * webarchive/loading/resources/test-loading-archive-main.webarchive: Copied from LayoutTests/webarchive/loading/test-loading-archive.html.
+        * webarchive/loading/test-loading-archive-subresource-expected.txt: Added.
+        * webarchive/loading/test-loading-archive-subresource-null-mimetype.html:
+        * webarchive/loading/test-loading-archive-subresource.html: Copied from LayoutTests/webarchive/loading/test-loading-archive.html.
+        * webarchive/loading/test-loading-archive.html:
+        * webarchive/loading/test-loading-top-archive-expected.txt: Added.
+        * webarchive/loading/test-loading-top-archive.html: Added.
+
 2018-11-27  Per Arne Vollan  <pvollan@apple.com>
 
         Layout Test svg/text/monospace-text-size-in-img.html is failing
diff --git a/LayoutTests/platform/mac-wk1/webarchive/loading/test-loading-archive-subresource-expected.txt b/LayoutTests/platform/mac-wk1/webarchive/loading/test-loading-archive-subresource-expected.txt
new file mode 100644 (file)
index 0000000..311f519
--- /dev/null
@@ -0,0 +1,8 @@
+main frame - didStartProvisionalLoadForFrame
+main frame - didCommitLoadForFrame
+frame "<!--frame1-->" - didStartProvisionalLoadForFrame
+main frame - didFinishDocumentLoadForFrame
+main frame - didHandleOnloadEventsForFrame
+frame "<!--frame1-->" - didFailProvisionalLoadWithError
+main frame - didFinishLoadForFrame
+ This test assumes the webarchive is not loaded.
index cb18e371961c2090b7b64fe0b2ee77e36055b4a0..727fc508d4c2afee2df13a9881cf332934f73065 100644 (file)
@@ -3,6 +3,8 @@ if (window.testRunner) {
        testRunner.dumpAsText();
        testRunner.dumpChildFramesAsText();
 }
+if (window.internals)
+    internals.setAlwaysAllowLocalWebarchive();
 </script>
 The webarchive in this iframe is utf-8 encoded and will only display properly if the webarchive's encoding is respected.<br>
 <iframe src="resources/utf8-encoded.webarchive"></iframe>
index b110ae8e424851a86ac16651a3b16c6574f4ffaf..20e9600463f6decb0feb8b351d142a0d16b59f88 100644 (file)
@@ -4,6 +4,8 @@
         testRunner.dumpResourceLoadCallbacks();
         testRunner.waitUntilDone();
     }
+    if (window.internals)
+        internals.setAlwaysAllowLocalWebarchive();
     
     function frameLoaded() {
         if (window.testRunner)
index 08db4c6f2a44e130a2d219380bb929a5365606e8..95411fe5badc412b6db610e3ae13e1e2c4f26158 100644 (file)
@@ -4,6 +4,8 @@
         testRunner.dumpAsText();
         testRunner.waitUntilDone();
     }
+    if (window.internals)
+        internals.setAlwaysAllowLocalWebarchive();
     onload = function() {
         frame = document.createElement("iframe");
         frame.src = "resources/mainresource-null-mimetype.webarchive";
index cc827be5de1b17b683d233e50ada2777880ac4f6..e34fd7bf6bcd1336af41f6954ea3a021e43f0e6d 100644 (file)
@@ -3,7 +3,8 @@
         testRunner.waitUntilDone();
         testRunner.dumpAsText();
     }
-
+    if (window.internals)
+        internals.setAlwaysAllowLocalWebarchive();
     onload = function() {
         frame = document.createElement("iframe");
         frame.src = "resources/missing-data.webarchive";
diff --git a/LayoutTests/webarchive/loading/resources/top.webarchive b/LayoutTests/webarchive/loading/resources/top.webarchive
new file mode 100644 (file)
index 0000000..c636935
Binary files /dev/null and b/LayoutTests/webarchive/loading/resources/top.webarchive differ
diff --git a/LayoutTests/webarchive/loading/test-loading-archive-subresource-expected.txt b/LayoutTests/webarchive/loading/test-loading-archive-subresource-expected.txt
new file mode 100644 (file)
index 0000000..a3faadf
--- /dev/null
@@ -0,0 +1,8 @@
+main frame - didStartProvisionalLoadForFrame
+main frame - didCommitLoadForFrame
+main frame - didFinishDocumentLoadForFrame
+frame "<!--frame1-->" - didStartProvisionalLoadForFrame
+main frame - didHandleOnloadEventsForFrame
+frame "<!--frame1-->" - didFailProvisionalLoadWithError
+main frame - didFinishLoadForFrame
+ This test assumes the webarchive is not loaded.
index f9765df2f06721d5a5f79666b253a4eda1aed6b5..23ce0036f432820c0066b8389c0cf87ae82dbb1b 100644 (file)
@@ -5,7 +5,8 @@
         testRunner.dumpAsText();
         testRunner.waitUntilDone();
     }
-    
+    if (window.internals)
+        internals.setAlwaysAllowLocalWebarchive();
     function frameLoaded() {
         if (window.testRunner)
             testRunner.notifyDone();
diff --git a/LayoutTests/webarchive/loading/test-loading-archive-subresource.html b/LayoutTests/webarchive/loading/test-loading-archive-subresource.html
new file mode 100644 (file)
index 0000000..123cffa
--- /dev/null
@@ -0,0 +1,18 @@
+<html>
+<script>
+    if (window.testRunner) {
+        testRunner.dumpAsText();
+        testRunner.waitUntilDone();
+    }
+
+    function frameLoaded() {
+        if (window.testRunner)
+            testRunner.notifyDone();
+    }
+    setTimeout(frameLoaded, 10);
+</script>
+<body>
+    <iframe onload="frameLoaded();" src="resources/helloworld.webarchive"></iframe>
+    This test assumes the webarchive is not loaded.
+</body>
+</html>
index 1ebc18a9ff0e5cc60361b70227d70623f40ec036..04b70e4371dae28962ad13faecfa64ca97cf8767 100644 (file)
@@ -5,7 +5,8 @@
         testRunner.dumpAsText();
         testRunner.waitUntilDone();
     }
-    
+    if (window.internals)
+        internals.setAlwaysAllowLocalWebarchive();
     function frameLoaded() {
         if (window.testRunner)
             testRunner.notifyDone();
diff --git a/LayoutTests/webarchive/loading/test-loading-top-archive-expected.txt b/LayoutTests/webarchive/loading/test-loading-top-archive-expected.txt
new file mode 100644 (file)
index 0000000..61959cd
--- /dev/null
@@ -0,0 +1,12 @@
+main frame - didStartProvisionalLoadForFrame
+main frame - didCommitLoadForFrame
+main frame - willPerformClientRedirectToURL: resources/top.webarchive 
+main frame - didFinishDocumentLoadForFrame
+main frame - didFinishLoadForFrame
+main frame - didStartProvisionalLoadForFrame
+main frame - didCancelClientRedirectForFrame
+main frame - didCommitLoadForFrame
+main frame - didFinishDocumentLoadForFrame
+main frame - didHandleOnloadEventsForFrame
+main frame - didFinishLoadForFrame
+hello, world.
diff --git a/LayoutTests/webarchive/loading/test-loading-top-archive.html b/LayoutTests/webarchive/loading/test-loading-top-archive.html
new file mode 100644 (file)
index 0000000..65db63f
--- /dev/null
@@ -0,0 +1,9 @@
+<html>
+<script>
+    if (window.testRunner) {
+        testRunner.waitUntilDone();
+        testRunner.dumpAsText();
+    }
+    window.location="resources/top.webarchive";
+</script>
+</html>
index 552f8b9c8f3d14c46282a274c3efb50a503e3326..7f91e620644dfc83a3b887e4107fb50de6f85777 100644 (file)
@@ -1,3 +1,33 @@
+2018-11-16  Jiewen Tan  <jiewen_tan@apple.com>
+
+        Disallow loading webarchives as iframes
+        https://bugs.webkit.org/show_bug.cgi?id=191728
+        <rdar://problem/45524528>
+
+        Reviewed by Youenn Fablet.
+
+        Disallow loading webarchives as iframes. We don't allow loading remote webarchives.
+        Now, this policy is hardened to disallow loading webarchives as iframes for local
+        documents as well.
+
+        To allow old tests still be able to run, a flag is added to always allow loading local
+        webarchives in document. The flag can be set via window.internals.
+
+        Tests: webarchive/loading/test-loading-archive-subresource.html
+               webarchive/loading/test-loading-top-archive.html
+
+        * dom/Document.h:
+        (WebCore::Document::setAlwaysAllowLocalWebarchive):
+        (WebCore::Document::alwaysAllowLocalWebarchive):
+        * loader/DocumentLoader.cpp:
+        (WebCore::disallowWebArchive):
+        (WebCore::DocumentLoader::continueAfterContentPolicy):
+        (WebCore::isRemoteWebArchive): Deleted.
+        * testing/Internals.cpp:
+        (WebCore::Internals::setAlwaysAllowLocalWebarchive const):
+        * testing/Internals.h:
+        * testing/Internals.idl:
+
 2018-11-27  Jer Noble  <jer.noble@apple.com>
 
         Unregister CDMSessionMediaSourceAVFObjC for error notifications during destruction.
index cd53dba685a9b030a342680d52f435bf5d3c9b13..559a5be63af393b25e90fb503549f2819e8a773e 100644 (file)
@@ -1530,6 +1530,10 @@ public:
 
     void frameWasDisconnectedFromOwner();
 
+    // Used in webarchive loading tests.
+    void setAlwaysAllowLocalWebarchive() { m_alwaysAllowLocalWebarchive = true; }
+    bool alwaysAllowLocalWebarchive() const { return m_alwaysAllowLocalWebarchive; }
+
 protected:
     enum ConstructionFlags { Synthesized = 1, NonRenderedPlaceholder = 1 << 1 };
     Document(Frame*, const URL&, unsigned = DefaultDocumentClass, unsigned constructionFlags = 0);
@@ -2069,6 +2073,8 @@ private:
 #endif
 
     bool m_isRunningUserScripts { false };
+
+    bool m_alwaysAllowLocalWebarchive { false };
 };
 
 Element* eventTargetElementForDocument(Document*);
index 3d5c3b37545b8aae273d573ddb5c6e76c531948c..d851af0ee6ce8e2cbb124630efbd6056a02885b8 100644 (file)
@@ -841,7 +841,9 @@ void DocumentLoader::responseReceived(const ResourceResponse& response, Completi
     });
 }
 
-static bool isRemoteWebArchive(const DocumentLoader& documentLoader)
+// Prevent web archives from loading if it is remote or it is not the main frame because they
+// can claim to be from any domain and thus avoid cross-domain security checks (4120255, 45524528).
+bool DocumentLoader::disallowWebArchive() const
 {
     using MIMETypeHashSet = HashSet<String, ASCIICaseInsensitiveHash>;
     static NeverDestroyed<MIMETypeHashSet> webArchiveMIMETypes {
@@ -855,17 +857,28 @@ static bool isRemoteWebArchive(const DocumentLoader& documentLoader)
         }
     };
 
-    const ResourceResponse& response = documentLoader.response();
-    String mimeType = response.mimeType();
+    String mimeType = m_response.mimeType();
     if (mimeType.isNull() || !webArchiveMIMETypes.get().contains(mimeType))
         return false;
 
 #if USE(QUICK_LOOK)
-    if (isQuickLookPreviewURL(response.url()))
+    if (isQuickLookPreviewURL(m_response.url()))
         return false;
 #endif
 
-    return !documentLoader.substituteData().isValid() && !SchemeRegistry::shouldTreatURLSchemeAsLocal(documentLoader.request().url().protocol().toStringWithoutCopying());
+    if (m_substituteData.isValid())
+        return false;
+
+    if (!SchemeRegistry::shouldTreatURLSchemeAsLocal(m_request.url().protocol().toStringWithoutCopying()))
+        return true;
+
+    if (!frame() || frame()->isMainFrame())
+        return false;
+
+    // On purpose of maintaining existing tests.
+    if (!frame()->document() || frame()->document()->topDocument().alwaysAllowLocalWebarchive())
+        return false;
+    return true;
 }
 
 void DocumentLoader::continueAfterContentPolicy(PolicyAction policy)
@@ -877,8 +890,7 @@ void DocumentLoader::continueAfterContentPolicy(PolicyAction policy)
 
     switch (policy) {
     case PolicyAction::Use: {
-        // Prevent remote web archives from loading because they can claim to be from any domain and thus avoid cross-domain security checks (4120255).
-        if (!frameLoader()->client().canShowMIMEType(m_response.mimeType()) || isRemoteWebArchive(*this)) {
+        if (!frameLoader()->client().canShowMIMEType(m_response.mimeType()) || disallowWebArchive()) {
             frameLoader()->policyChecker().cannotShowMIMEType(m_response);
             // Check reachedTerminalState since the load may have already been canceled inside of _handleUnimplementablePolicyWithErrorCode::.
             stopLoadingForPolicyChange();
index c2c8cc72e8f0aeea658dfb524de445716febeedf..b59f4bbe92cb79dbd476b3d7486520d51cb7ccfb 100644 (file)
@@ -414,6 +414,8 @@ private:
     WEBCORE_EXPORT void sendCSPViolationReport(URL&&, Ref<FormData>&&) final;
     WEBCORE_EXPORT void enqueueSecurityPolicyViolationEvent(SecurityPolicyViolationEvent::Init&&) final;
 
+    bool disallowWebArchive() const;
+
     Ref<CachedResourceLoader> m_cachedResourceLoader;
 
     CachedResourceHandle<CachedRawResource> m_mainResource;
index 1b5db019d26d7cade3be0bcc12e4e77b52aa8ae3..2fb676d4fb991a388b85943bb79041e87489f6b8 100644 (file)
@@ -4824,4 +4824,12 @@ auto Internals::getCookies() const -> Vector<CookieData>
     });
 }
 
+void Internals::setAlwaysAllowLocalWebarchive() const
+{
+    auto* document = contextDocument();
+    if (!document)
+        return;
+    document->setAlwaysAllowLocalWebarchive();
+}
+
 } // namespace WebCore
index caeb9753f728945d620f8cbb8295c37ce8386241..943967f0da878f1e835d46f4e13e140c79139e07 100644 (file)
@@ -785,6 +785,8 @@ public:
     };
     Vector<CookieData> getCookies() const;
 
+    void setAlwaysAllowLocalWebarchive() const;
+
 private:
     explicit Internals(Document&);
     Document* contextDocument() const;
index ef8a18e4eddd05fe6290009e33193d6a5f0c8b09..a1705edcbdf273510a0cf343e07d2ed7841af206 100644 (file)
@@ -723,4 +723,6 @@ enum CompositingPolicy {
     HEVCParameterSet? parseHEVCCodecParameters(DOMString codecParameters);
 
     sequence<CookieData> getCookies();
+
+    void setAlwaysAllowLocalWebarchive();
 };
index 655df65af691088287ef92985187f61d534ca88c..d53254964a99037068c737f11b1bc2de28a32469 100644 (file)
@@ -1,3 +1,15 @@
+2018-11-16  Jiewen Tan  <jiewen_tan@apple.com>
+
+        Disallow loading webarchives as iframes
+        https://bugs.webkit.org/show_bug.cgi?id=191728
+        <rdar://problem/45524528>
+
+        Reviewed by Youenn Fablet.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::unableToImplementPolicy):
+        Add a check to prevent null pointer dereference.
+
 2018-11-27  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         WebKit.AddAndRemoveDataDetectors hits a debug assertion after r238515
index 0827b02273f906e73c7efe199975baa9dc6cc6f0..77608c70dcd4685ff2e7a3904167640b3a1a26be 100644 (file)
@@ -4414,6 +4414,8 @@ void WebPageProxy::unableToImplementPolicy(uint64_t frameID, const ResourceError
     WebFrameProxy* frame = m_process->webFrame(frameID);
     MESSAGE_CHECK(frame);
 
+    if (!m_policyClient)
+        return;
     m_policyClient->unableToImplementPolicy(*this, *frame, error, m_process->transformHandlesToObjects(userData.object()).get());
 }