Bug 1517301 - Move CSP check for form-action to be within HTMLFormSubmission to prevent checking before the form should be submitted. r=ckerschb,smaug
authorJonathan Kingston <jkt@mozilla.com>
Wed, 09 Jan 2019 15:42:04 +0000
changeset 510161 45bb6ff923e9fb36b4a53cbdfa99d360e2d3e1a8
parent 510160 52bc1477d801eebf61511e0071edc949dcdc1a36
child 510162 039e8cac38fd565ee41b67f36ff4529d6df211d9
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersckerschb, smaug
bugs1517301
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1517301 - Move CSP check for form-action to be within HTMLFormSubmission to prevent checking before the form should be submitted. r=ckerschb,smaug Differential Revision: https://phabricator.services.mozilla.com/D16032
dom/html/HTMLFormElement.cpp
dom/html/HTMLFormSubmission.cpp
testing/web-platform/tests/content-security-policy/form-action/form-action-src-javascript-prevented.html
--- a/dom/html/HTMLFormElement.cpp
+++ b/dom/html/HTMLFormElement.cpp
@@ -1554,35 +1554,16 @@ nsresult HTMLFormElement::GetActionURL(n
   // Get security manager, check to see if access to action URI is allowed.
   //
   nsIScriptSecurityManager* securityManager =
       nsContentUtils::GetSecurityManager();
   rv = securityManager->CheckLoadURIWithPrincipal(
       NodePrincipal(), actionURL, nsIScriptSecurityManager::STANDARD);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  // Check if CSP allows this form-action
-  nsCOMPtr<nsIContentSecurityPolicy> csp;
-  rv = NodePrincipal()->GetCsp(getter_AddRefs(csp));
-  NS_ENSURE_SUCCESS(rv, rv);
-  if (csp) {
-    bool permitsFormAction = true;
-
-    // form-action is only enforced if explicitly defined in the
-    // policy - do *not* consult default-src, see:
-    // http://www.w3.org/TR/CSP2/#directive-default-src
-    rv = csp->Permits(this, nullptr /* nsICSPEventListener */, actionURL,
-                      nsIContentSecurityPolicy::FORM_ACTION_DIRECTIVE, true,
-                      &permitsFormAction);
-    NS_ENSURE_SUCCESS(rv, rv);
-    if (!permitsFormAction) {
-      return NS_ERROR_CSP_FORM_ACTION_VIOLATION;
-    }
-  }
-
   // Potentially the page uses the CSP directive 'upgrade-insecure-requests'. In
   // such a case we have to upgrade the action url from http:// to https://.
   // If the actionURL is not http, then there is nothing to do.
   bool isHttpScheme = false;
   rv = actionURL->SchemeIs("http", &isHttpScheme);
   NS_ENSURE_SUCCESS(rv, rv);
   if (isHttpScheme && document->GetUpgradeInsecureRequests(false)) {
     // let's use the old specification before the upgrade for logging
--- a/dom/html/HTMLFormSubmission.cpp
+++ b/dom/html/HTMLFormSubmission.cpp
@@ -818,16 +818,35 @@ void GetEnumAttr(nsGenericHTMLElement* a
 
   nsresult rv;
 
   // Get action
   nsCOMPtr<nsIURI> actionURL;
   rv = aForm->GetActionURL(getter_AddRefs(actionURL), aOriginatingElement);
   NS_ENSURE_SUCCESS(rv, rv);
 
+ // Check if CSP allows this form-action
+ nsCOMPtr<nsIContentSecurityPolicy> csp;
+ rv = aForm->NodePrincipal()->GetCsp(getter_AddRefs(csp));
+ NS_ENSURE_SUCCESS(rv, rv);
+ if (csp) {
+   bool permitsFormAction = true;
+
+   // form-action is only enforced if explicitly defined in the
+   // policy - do *not* consult default-src, see:
+   // http://www.w3.org/TR/CSP2/#directive-default-src
+   rv = csp->Permits(aForm, nullptr /* nsICSPEventListener */, actionURL,
+                     nsIContentSecurityPolicy::FORM_ACTION_DIRECTIVE, true,
+                     &permitsFormAction);
+   NS_ENSURE_SUCCESS(rv, rv);
+   if (!permitsFormAction) {
+     return NS_ERROR_CSP_FORM_ACTION_VIOLATION;
+   }
+ }
+
   // Get target
   // The target is the originating element formtarget attribute if the element
   // is a submit control and has such an attribute.
   // Otherwise, the target is the form owner's target attribute,
   // if it has such an attribute.
   // Finally, if one of the child nodes of the head element is a base element
   // with a target attribute, then the value of the target attribute of the
   // first such base element; or, if there is no such element, the empty string.
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/content-security-policy/form-action/form-action-src-javascript-prevented.html
@@ -0,0 +1,46 @@
+<!DOCTYPE html>
+<html>
+
+<head>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<meta http-equiv="Content-Security-Policy" content="form-action 'none'; script-src 'self' 'nonce-noncynonce'; connect-src 'self';">
+</head>
+
+<body>
+  <form action='/content-security-policy/support/postmessage-pass-to-opener.html'
+        id='form_id'
+        target="_blank">
+        <input type="submit" />
+  </form>
+
+  <p>
+    Test that "form-action 'none'" doesn't create a violation report if the event was prevented.
+  </p>
+</body>
+
+<script nonce='noncynonce'>
+  async_test(t => {
+    document.addEventListener('securitypolicyviolation', function(e) {
+      assert_unreached('Form submission was blocked.');
+    });
+
+    window.addEventListener('message', function(event) {
+      assert_unreached('Form submission was blocked.');
+    })
+
+    window.addEventListener("load", function() {
+      let form = document.getElementById("form_id");
+      form.addEventListener("submit", e => {
+        e.preventDefault();
+        setTimeout(() => {
+          t.done();
+        }, 0);
+      });
+      // clicking the input is used here as form.submit() will submit a form without an event and should also be blocked.
+      form.querySelector("input").click();
+    });
+  }, "The form submission should not be blocked by when javascript prevents the load.");
+</script>
+
+</html>