Changeset 89155 in webkit

Timestamp:

Jun 17, 2011 12:11:14 PM (13 years ago)

Author:

commit-queue@webkit.org

Message:

2011-06-17 Chris Evans <​cevans@chromium.org>

Reviewed by Adam Barth.

Detect mixed-scripting involving https -> http redirects
​https://bugs.webkit.org/show_bug.cgi?id=62846

• http/tests/security/mixedContent/redirect-https-to-http-script-in-iframe-expected.txt: Update expectation.
• http/tests/security/mixedContent/redirect-https-to-http-script-in-iframe.html: Remove FIXME as the test is now working as expected.

2011-06-17 Chris Evans <​cevans@chromium.org>

Reviewed by Adam Barth.

Detect mixed-scripting involving https -> http redirects
​https://bugs.webkit.org/show_bug.cgi?id=62846

Test: http/tests/security/mixedContent/redirect-https-to-http-script-in-iframe.html

• loader/cache/CachedResourceLoader.cpp: (WebCore::CachedResourceLoader::checkInsecureContent): (WebCore::CachedResourceLoader::canRequest): break out insecure content logic.
• loader/cache/CachedResourceLoader.h:
• loader/cache/CachedResourceRequest.cpp: (WebCore::CachedResourceRequest::willSendRequest): check the redirect target for possible insecure content issues.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/http/tests/security/mixedContent/redirect-https-to-http-script-in-iframe-expected.txt (modified) (2 diffs)
• LayoutTests/http/tests/security/mixedContent/redirect-https-to-http-script-in-iframe.html (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/loader/cache/CachedResourceLoader.cpp (modified) (2 diffs)
• Source/WebCore/loader/cache/CachedResourceLoader.h (modified) (1 diff)
• Source/WebCore/loader/cache/CachedResourceRequest.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2011-06-17  Chris Evans  <cevans@chromium.org>
+
+        Reviewed by Adam Barth.
+
+        Detect mixed-scripting involving https -> http redirects
+        https://bugs.webkit.org/show_bug.cgi?id=62846
+
+        * http/tests/security/mixedContent/redirect-https-to-http-script-in-iframe-expected.txt: Update expectation.
+        * http/tests/security/mixedContent/redirect-https-to-http-script-in-iframe.html: Remove FIXME as the test is now working as expected.
+
 2011-06-12  Robert Hogan  <robert@webkit.org>
 

trunk/LayoutTests/http/tests/security/mixedContent/redirect-https-to-http-script-in-iframe-expected.txt

@@ -2 +2 @@
 main frame - didFinishDocumentLoadForFrame
 frame "<!--framePath //<!--frame0-->-->" - didCommitLoadForFrame
+CONSOLE MESSAGE: line 1: The page at https://127.0.0.1:8443/security/mixedContent/resources/frame-with-redirect-https-to-http-script.html ran insecure content from http://127.0.0.1:8080/security/mixedContent/resources/script.js.
+
+didRunInsecureContent
 frame "<!--framePath //<!--frame0-->-->" - didFinishDocumentLoadForFrame
 frame "<!--framePath //<!--frame0-->-->" - didHandleOnloadEventsForFrame
@@ -9 +12 @@
 This test loads a secure iframe that loads an insecure script (but with a tricky redirect). We should trigger a mixed content callback because an active network attacker can end up controling the script.
 
-FIXME: This test current does not trigger a mixed content callback!
 
-

trunk/LayoutTests/http/tests/security/mixedContent/redirect-https-to-http-script-in-iframe.html

@@ -10 +10 @@
 tricky redirect).  We should trigger a mixed content callback because an active
 network attacker can end up controling the script.</p>
-
-<p>FIXME: This test current does not trigger a mixed content callback!</p>
 <iframe src="https://127.0.0.1:8443/security/mixedContent/resources/frame-with-redirect-https-to-http-script.html";
 ></iframe>

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-06-17  Chris Evans  <cevans@chromium.org>
+
+        Reviewed by Adam Barth.
+
+        Detect mixed-scripting involving https -> http redirects
+        https://bugs.webkit.org/show_bug.cgi?id=62846
+
+        Test: http/tests/security/mixedContent/redirect-https-to-http-script-in-iframe.html
+
+        * loader/cache/CachedResourceLoader.cpp:
+        (WebCore::CachedResourceLoader::checkInsecureContent):
+        (WebCore::CachedResourceLoader::canRequest): break out insecure content logic.
+        * loader/cache/CachedResourceLoader.h:
+        * loader/cache/CachedResourceRequest.cpp:
+        (WebCore::CachedResourceRequest::willSendRequest): check the redirect target for possible insecure content issues.
+
 2011-06-12  Robert Hogan  <robert@webkit.org>
 

trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp

@@ -200 +200 @@
 #endif
 
+bool CachedResourceLoader::checkInsecureContent(CachedResource::Type type, const KURL& url) const
+{
+    switch (type) {
+    case CachedResource::Script:
+#if ENABLE(XSLT)
+    case CachedResource::XSLStyleSheet:
+#endif
+    case CachedResource::CSSStyleSheet:
+        // These resource can inject script into the current document (Script,
+        // XSL) or exfiltrate the content of the current document (CSS).
+        if (Frame* f = frame())
+            if (!f->loader()->checkIfRunInsecureContent(m_document->securityOrigin(), url))
+                return false;
+        break;
+    case CachedResource::ImageResource:
+    case CachedResource::FontResource: {
+        // These resources can corrupt only the frame's pixels.
+        if (Frame* f = frame()) {
+            Frame* top = f->tree()->top();
+            if (!top->loader()->checkIfDisplayInsecureContent(top->document()->securityOrigin(), url))
+                return false;
+        }
+        break;
+    }
+#if ENABLE(LINK_PREFETCH)
+    case CachedResource::LinkPrefetch:
+    case CachedResource::LinkPrerender:
+    case CachedResource::LinkSubresource:
+        // Prefetch cannot affect the current document.
+        break;
+#endif
+    }
+    return true;
+}
+
 bool CachedResourceLoader::canRequest(CachedResource::Type type, const KURL& url, bool forPreload)
 {
@@ -238 +273 @@
     // check whether the load passes the mixed-content policy.
     //
-    // Note: Currently, we always allow mixed content, but we generate a
-    //       callback to the FrameLoaderClient in case the embedder wants to
-    //       update any security indicators.
-    // 
     // FIXME: Should we consider forPreload here?
-    //
-    switch (type) {
-    case CachedResource::Script:
-#if ENABLE(XSLT)
-    case CachedResource::XSLStyleSheet:
-#endif
-    case CachedResource::CSSStyleSheet:
-        // These resource can inject script into the current document (Script,
-        // XSL) or exfiltrate the content of the current document (CSS).
-        if (Frame* f = frame())
-            if (!f->loader()->checkIfRunInsecureContent(m_document->securityOrigin(), url))
-                return false;
-        break;
-    case CachedResource::ImageResource:
-    case CachedResource::FontResource: {
-        // These resources can corrupt only the frame's pixels.
-        if (Frame* f = frame()) {
-            Frame* top = f->tree()->top();
-            if (!top->loader()->checkIfDisplayInsecureContent(top->document()->securityOrigin(), url))
-                return false;
-        }
-        break;
-    }
-#if ENABLE(LINK_PREFETCH)
-    case CachedResource::LinkPrefetch:
-    case CachedResource::LinkPrerender:
-    case CachedResource::LinkSubresource:
-        // Prefetch cannot affect the current document.
-        break;
-#endif
-    }
+    if (!checkInsecureContent(type, url))
+        return false;
+
     // FIXME: Consider letting the embedder block mixed content loads.
 

trunk/Source/WebCore/loader/cache/CachedResourceLoader.h

@@ -103 +103 @@
     void checkForPendingPreloads();
     void printPreloadStats();
+    bool checkInsecureContent(CachedResource::Type, const KURL&) const;
     
 private:

trunk/Source/WebCore/loader/cache/CachedResourceRequest.cpp

@@ -135 +135 @@
 }
 
-void CachedResourceRequest::willSendRequest(SubresourceLoader*, ResourceRequest&, const ResourceResponse&)
-{
+void CachedResourceRequest::willSendRequest(SubresourceLoader* loader, ResourceRequest& req, const ResourceResponse&)
+{
+    if (!m_cachedResourceLoader->checkInsecureContent(m_resource->type(), req.url())) {
+        loader->cancel();
+        return;
+    }
     m_resource->setRequestedFromNetworkingLayer();
 }