CSP: Check <param> element values against the document's CSP before l…

…oading. We ought to take account of the 'param' element parsing behavior that happens in 'HTMLObjectElement'. This patch moves the pluginIsLoadable check to make that happen. To avoid 'setTimeout' in the test, and to align with the spec[1], this patch also starts dispatching an 'error' event on load failure for 'object' elements. [1]: #4.6 ("If the load failed...") of http://www.whatwg.org/specs/web-apps/current-work/multipage/the-iframe-element.html#the-object-element BUG=320796 [email protected],[email protected] Review URL: https://codereview.chromium.org/76303002 git-svn-id: svn://svn.chromium.org/blink/trunk@164952 bbb929c8-8fbe-4397-9dbb-9b2b20218538
yoavweiss · Jan 13, 2014 · b0e15a6 · b0e15a6
1 parent a1f451a
commit b0e15a6
Show file tree

Hide file tree

Showing 13 changed files with 125 additions and 14 deletions.
diff --git a/...utTests/http/tests/security/contentSecurityPolicy/1.1/plugintypes-notype-url-expected.txt b/...utTests/http/tests/security/contentSecurityPolicy/1.1/plugintypes-notype-url-expected.txt
@@ -1,8 +1,3 @@
 CONSOLE ERROR: Refused to load 'http://127.0.0.1:8000/plugins/resources/mock-plugin.pl' (MIME type '') because it violates the following Content Security Policy Directive: 'plugin-types application/x-invalid-type'. When enforcing the 'plugin-types' directive, the plugin's media type must be explicitly declared with a 'type' attribute on the containing element (e.g. '<object type="[TYPE GOES HERE]" ...>').
 
 Given a `plugin-types` directive, plugins have to declare a type explicitly. No declared type, no load. This test passes if there's a console message above.  
-
---------
-Frame: '<!--framePath //<!--frame0-->-->'
---------
-
diff --git a/...ests/http/tests/security/contentSecurityPolicy/object-src-param-code-blocked-expected.txt b/...ests/http/tests/security/contentSecurityPolicy/object-src-param-code-blocked-expected.txt
@@ -0,0 +1,4 @@
+CONSOLE ERROR: Refused to load plugin data from 'http://127.0.0.1:8080/plugins/resources/mock-plugin.pl?code' because it violates the following Content Security Policy directive: "object-src http://localhost:8080".
+
+CONSOLE MESSAGE: line 16: PASS: Error occurred, so load was correctly blocked.
+This test passes if there is a console message saying the plugin was blocked.  
diff --git a/LayoutTests/http/tests/security/contentSecurityPolicy/object-src-param-code-blocked.html b/LayoutTests/http/tests/security/contentSecurityPolicy/object-src-param-code-blocked.html
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="resources/object-src-param.js"></script>
+<meta http-equiv="Content-Security-Policy" content="object-src http://localhost:8080">
+</head>
+<body>
+This test passes if there is a console message saying the plugin was blocked.
+<script>
+    appendObjectElement('code');
+</script>
+</body>
+</html>
diff --git a/...sts/http/tests/security/contentSecurityPolicy/object-src-param-movie-blocked-expected.txt b/...sts/http/tests/security/contentSecurityPolicy/object-src-param-movie-blocked-expected.txt
@@ -0,0 +1,4 @@
+CONSOLE ERROR: Refused to load plugin data from 'http://127.0.0.1:8080/plugins/resources/mock-plugin.pl?movie' because it violates the following Content Security Policy directive: "object-src http://localhost:8080".
+
+CONSOLE MESSAGE: line 16: PASS: Error occurred, so load was correctly blocked.
+This test passes if there is a console message saying the plugin was blocked.  
diff --git a/LayoutTests/http/tests/security/contentSecurityPolicy/object-src-param-movie-blocked.html b/LayoutTests/http/tests/security/contentSecurityPolicy/object-src-param-movie-blocked.html
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="resources/object-src-param.js"></script>
+<meta http-equiv="Content-Security-Policy" content="object-src http://localhost:8080">
+</head>
+<body>
+This test passes if there is a console message saying the plugin was blocked.
+<script>
+    appendObjectElement('movie');
+</script>
+</body>
+</html>
diff --git a/...Tests/http/tests/security/contentSecurityPolicy/object-src-param-src-blocked-expected.txt b/...Tests/http/tests/security/contentSecurityPolicy/object-src-param-src-blocked-expected.txt
@@ -0,0 +1,4 @@
+CONSOLE ERROR: Refused to load plugin data from 'http://127.0.0.1:8080/plugins/resources/mock-plugin.pl?src' because it violates the following Content Security Policy directive: "object-src http://localhost:8080".
+
+CONSOLE MESSAGE: line 16: PASS: Error occurred, so load was correctly blocked.
+This test passes if there is a console message saying the plugin was blocked.  
diff --git a/LayoutTests/http/tests/security/contentSecurityPolicy/object-src-param-src-blocked.html b/LayoutTests/http/tests/security/contentSecurityPolicy/object-src-param-src-blocked.html
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="resources/object-src-param.js"></script>
+<meta http-equiv="Content-Security-Policy" content="object-src http://localhost:8080">
+</head>
+<body>
+This test passes if there is a console message saying the plugin was blocked.
+<script>
+    appendObjectElement('src');
+</script>
+</body>
+</html>
diff --git a/...Tests/http/tests/security/contentSecurityPolicy/object-src-param-url-blocked-expected.txt b/...Tests/http/tests/security/contentSecurityPolicy/object-src-param-url-blocked-expected.txt
@@ -0,0 +1,4 @@
+CONSOLE ERROR: Refused to load plugin data from 'http://127.0.0.1:8080/plugins/resources/mock-plugin.pl?url' because it violates the following Content Security Policy directive: "object-src http://localhost:8080".
+
+CONSOLE MESSAGE: line 16: PASS: Error occurred, so load was correctly blocked.
+This test passes if there is a console message saying the plugin was blocked.  
diff --git a/LayoutTests/http/tests/security/contentSecurityPolicy/object-src-param-url-blocked.html b/LayoutTests/http/tests/security/contentSecurityPolicy/object-src-param-url-blocked.html
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="resources/object-src-param.js"></script>
+<meta http-equiv="Content-Security-Policy" content="object-src http://localhost:8080">
+</head>
+<body>
+This test passes if there is a console message saying the plugin was blocked.
+<script>
+    appendObjectElement('url');
+</script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/security/contentSecurityPolicy/resources/object-src-param.js b/LayoutTests/http/tests/security/contentSecurityPolicy/resources/object-src-param.js
@@ -0,0 +1,29 @@
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+
+function appendObjectElement(type) {
+    window.onload = function () {
+        var o = document.createElement('object');
+        o.setAttribute('type', 'application/x-webkit-test-netscape');
+        o.addEventListener('load', function () {
+            console.log('FAIL: The object should have been blocked.');
+            if (window.testRunner)
+                testRunner.notifyDone();
+        });
+        o.addEventListener('error', function () {
+            console.log('PASS: Error occurred, so load was correctly blocked.');
+            if (window.testRunner)
+                testRunner.notifyDone();
+        });
+
+        var p = document.createElement('param');
+        p.setAttribute('value', 'http://127.0.0.1:8080/plugins/resources/mock-plugin.pl?' + type);
+        p.setAttribute('name', type);
+
+        o.appendChild(p);
+
+        document.body.appendChild(o);
+    };
+}
diff --git a/Source/core/html/HTMLObjectElement.cpp b/Source/core/html/HTMLObjectElement.cpp
@@ -270,14 +270,18 @@ void HTMLObjectElement::updateWidgetInternal()
     ASSERT(needsWidgetUpdate());
     setNeedsWidgetUpdate(false);
     // FIXME: This should ASSERT isFinishedParsingChildren() instead.
-    if (!isFinishedParsingChildren())
+    if (!isFinishedParsingChildren()) {
+        dispatchErrorEvent();
         return;
+    }
 
     // FIXME: I'm not sure it's ever possible to get into updateWidget during a
     // removal, but just in case we should avoid loading the frame to prevent
     // security bugs.
-    if (!SubframeLoadingDisabler::canLoadFrame(*this))
+    if (!SubframeLoadingDisabler::canLoadFrame(*this)) {
+        dispatchErrorEvent();
         return;
+    }
 
     String url = this->url();
     String serviceType = m_serviceType;
@@ -288,8 +292,10 @@ void HTMLObjectElement::updateWidgetInternal()
     parametersForPlugin(paramNames, paramValues, url, serviceType);
 
     // Note: url is modified above by parametersForPlugin.
-    if (!allowedToLoadFrameURL(url))
+    if (!allowedToLoadFrameURL(url)) {
+        dispatchErrorEvent();
         return;
+    }
 
     bool fallbackContent = hasFallbackContent();
     renderEmbeddedObject()->setHasFallbackContent(fallbackContent);
@@ -299,9 +305,12 @@ void HTMLObjectElement::updateWidgetInternal()
     if (!renderer()) // Do not load the plugin if beforeload removed this element or its renderer.
         return;
 
-    bool success = beforeLoadAllowedLoad && hasValidClassId() && requestObject(url, serviceType, paramNames, paramValues);
-    if (!success && fallbackContent)
-        renderFallbackContent();
+    if (!beforeLoadAllowedLoad || !hasValidClassId() || !requestObject(url, serviceType, paramNames, paramValues)) {
+        if (!url.isEmpty())
+            dispatchErrorEvent();
+        if (fallbackContent)
+            renderFallbackContent();
+    }
 }
 
 bool HTMLObjectElement::rendererIsNeeded(const RenderStyle& style)

diff --git a/Source/core/html/HTMLPlugInElement.cpp b/Source/core/html/HTMLPlugInElement.cpp
@@ -28,6 +28,7 @@
 #include "bindings/v8/ScriptController.h"
 #include "bindings/v8/npruntime_impl.h"
 #include "core/dom/Document.h"
+#include "core/dom/Node.h"
 #include "core/dom/PostAttachCallbacks.h"
 #include "core/dom/shadow/ShadowRoot.h"
 #include "core/events/Event.h"
@@ -410,6 +411,8 @@ bool HTMLPlugInElement::requestObject(const String& url, const String& mimeType,
         return false;
 
     KURL completedURL = document().completeURL(url);
+    if (!pluginIsLoadable(completedURL, mimeType))
+        return false;
 
     bool useFallback;
     if (shouldUsePlugin(completedURL, mimeType, renderer->hasFallbackContent(), useFallback))
@@ -429,9 +432,6 @@ bool HTMLPlugInElement::loadPlugin(const KURL& url, const String& mimeType, cons
     if (!frame->loader().allowPlugins(AboutToInstantiatePlugin))
         return false;
 
-    if (!pluginIsLoadable(url, mimeType))
-        return false;
-
     RenderEmbeddedObject* renderer = renderEmbeddedObject();
     // FIXME: This code should not depend on renderer!
     if (!renderer || useFallback)
@@ -477,6 +477,14 @@ bool HTMLPlugInElement::shouldUsePlugin(const KURL& url, const String& mimeType,
 
 }
 
+void HTMLPlugInElement::dispatchErrorEvent()
+{
+    if (document().isPluginDocument() && document().ownerElement())
+        document().ownerElement()->dispatchEvent(Event::create(EventTypeNames::error));
+    else
+        dispatchEvent(Event::create(EventTypeNames::error));
+}
+
 bool HTMLPlugInElement::pluginIsLoadable(const KURL& url, const String& mimeType)
 {
     Frame* frame = document().frame();

diff --git a/Source/core/html/HTMLPlugInElement.h b/Source/core/html/HTMLPlugInElement.h
@@ -79,6 +79,8 @@ class HTMLPlugInElement : public HTMLFrameOwnerElement {
     bool requestObject(const String& url, const String& mimeType, const Vector<String>& paramNames, const Vector<String>& paramValues);
     bool shouldUsePlugin(const KURL&, const String& mimeType, bool hasFallback, bool& useFallback);
 
+    void dispatchErrorEvent();
+
     String m_serviceType;
     String m_url;
     KURL m_loadedUrl;