Bug 1236222 - CSP: Blocked URI should be empty for inline violations. r=ckerschb
authorJonathan Kingston <jkt@mozilla.com>
Thu, 08 Mar 2018 16:23:03 -0800
changeset 411702 0e90c91234260397b7e817a3951108083ff039ea
parent 411701 7462fb5a5bdb965aa88db8887157060d86f004d3
child 411703 71c9c86c3c8d792a83eb0403fc06c8b249302840
push id101729
push usercsabou@mozilla.com
push dateWed, 04 Apr 2018 18:07:35 +0000
treeherdermozilla-inbound@3c240f56a113 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersckerschb
bugs1236222
milestone61.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 1236222 - CSP: Blocked URI should be empty for inline violations. r=ckerschb MozReview-Commit-ID: 6bMAVJl9RTG
dom/security/nsCSPContext.cpp
dom/security/nsCSPContext.h
dom/security/test/csp/test_report.html
dom/security/test/unit/test_csp_reports.js
toolkit/components/extensions/test/xpcshell/test_ext_contentscript_triggeringPrincipal.js
--- a/dom/security/nsCSPContext.cpp
+++ b/dom/security/nsCSPContext.cpp
@@ -892,17 +892,17 @@ StripURIForReporting(nsIURI* aURI,
   }
 
   // 3) Return uri, with any fragment component removed.
   aURI->GetSpecIgnoringRef(outStrippedURI);
 }
 
 nsresult
 nsCSPContext::GatherSecurityPolicyViolationEventData(
-  nsISupports* aBlockedContentSource,
+  nsIURI* aBlockedURI,
   nsIURI* aOriginalURI,
   nsAString& aViolatedDirective,
   uint32_t aViolatedPolicyIndex,
   nsAString& aSourceFile,
   nsAString& aScriptSample,
   uint32_t aLineNum,
   mozilla::dom::SecurityPolicyViolationEventInit& aViolationEventInit)
 {
@@ -916,33 +916,19 @@ nsCSPContext::GatherSecurityPolicyViolat
   nsAutoCString reportDocumentURI;
   StripURIForReporting(mSelfURI, mSelfURI, reportDocumentURI);
   aViolationEventInit.mDocumentURI = NS_ConvertUTF8toUTF16(reportDocumentURI);
 
   // referrer
   aViolationEventInit.mReferrer = mReferrer;
 
   // blocked-uri
-  if (aBlockedContentSource) {
+  if (aBlockedURI) {
     nsAutoCString reportBlockedURI;
-    nsCOMPtr<nsIURI> uri = do_QueryInterface(aBlockedContentSource);
-    // could be a string or URI
-    if (uri) {
-      StripURIForReporting(uri, mSelfURI, reportBlockedURI);
-    } else {
-      nsCOMPtr<nsISupportsCString> cstr = do_QueryInterface(aBlockedContentSource);
-      if (cstr) {
-        cstr->GetData(reportBlockedURI);
-      }
-    }
-    if (reportBlockedURI.IsEmpty()) {
-      // this can happen for frame-ancestors violation where the violating
-      // ancestor is cross-origin.
-      NS_WARNING("No blocked URI (null aBlockedContentSource) for CSP violation report.");
-    }
+    StripURIForReporting(aBlockedURI, mSelfURI, reportBlockedURI);
     aViolationEventInit.mBlockedURI = NS_ConvertUTF8toUTF16(reportBlockedURI);
   }
 
   // effective-directive
   // The name of the policy directive that was violated.
   aViolationEventInit.mEffectiveDirective = aViolatedDirective;
 
   // violated-directive
@@ -1261,18 +1247,20 @@ class CSPReportSenderRunnable final : pu
     NS_IMETHOD Run() override
     {
       MOZ_ASSERT(NS_IsMainThread());
 
       nsresult rv;
 
       // 0) prepare violation data
       mozilla::dom::SecurityPolicyViolationEventInit init;
+      // mBlockedContentSource could be a URI or a string.
+      nsCOMPtr<nsIURI> blockedURI = do_QueryInterface(mBlockedContentSource);
       rv = mCSPContext->GatherSecurityPolicyViolationEventData(
-        mBlockedContentSource, mOriginalURI,
+        blockedURI, mOriginalURI,
         mViolatedDirective, mViolatedPolicyIndex,
         mSourceFile, mScriptSample, mLineNum,
         init);
       NS_ENSURE_SUCCESS(rv, rv);
 
       // 1) notify observers
       nsCOMPtr<nsIObserverService> observerService = mozilla::services::GetObserverService();
       NS_ASSERTION(observerService, "needs observer service");
@@ -1280,18 +1268,16 @@ class CSPReportSenderRunnable final : pu
                                                      CSP_VIOLATION_TOPIC,
                                                      mViolatedDirective.get());
       NS_ENSURE_SUCCESS(rv, rv);
 
       // 2) send reports for the policy that was violated
       mCSPContext->SendReports(init, mViolatedPolicyIndex);
 
       // 3) log to console (one per policy violation)
-      // mBlockedContentSource could be a URI or a string.
-      nsCOMPtr<nsIURI> blockedURI = do_QueryInterface(mBlockedContentSource);
       // if mBlockedContentSource is not a URI, it could be a string
       nsCOMPtr<nsISupportsCString> blockedString = do_QueryInterface(mBlockedContentSource);
 
       nsCString blockedDataStr;
 
       if (blockedURI) {
         blockedURI->GetSpec(blockedDataStr);
         if (blockedDataStr.Length() > 40) {
--- a/dom/security/nsCSPContext.h
+++ b/dom/security/nsCSPContext.h
@@ -59,34 +59,33 @@ class nsCSPContext : public nsIContentSe
                       uint32_t aColumnNumber,
                       uint32_t aSeverityFlag);
 
 
 
     /**
      * Construct SecurityPolicyViolationEventInit structure.
      *
-     * @param aBlockedContentSource
-     *        Either a CSP Source (like 'self', as string) or nsIURI: the source
-     *        of the violation.
+     * @param aBlockedURI
+     *        A nsIURI: the source of the violation.
      * @param aOriginalUri
      *        The original URI if the blocked content is a redirect, else null
      * @param aViolatedDirective
      *        the directive that was violated (string).
      * @param aSourceFile
      *        name of the file containing the inline script violation
      * @param aScriptSample
      *        a sample of the violating inline script
      * @param aLineNum
      *        source line number of the violation (if available)
      * @param aViolationEventInit
      *        The output
      */
     nsresult GatherSecurityPolicyViolationEventData(
-      nsISupports* aBlockedContentSource,
+      nsIURI* aBlockedURI,
       nsIURI* aOriginalURI,
       nsAString& aViolatedDirective,
       uint32_t aViolatedPolicyIndex,
       nsAString& aSourceFile,
       nsAString& aScriptSample,
       uint32_t aLineNum,
       mozilla::dom::SecurityPolicyViolationEventInit& aViolationEventInit);
 
--- a/dom/security/test/csp/test_report.html
+++ b/dom/security/test/csp/test_report.html
@@ -43,17 +43,17 @@ window.checkResults = function(reportObj
   //    * source-file
   // see http://www.w3.org/TR/CSP11/#violation-reports
   is(cspReport["document-uri"], docUri, "Incorrect document-uri");
 
   // we can not test for the whole referrer since it includes platform specific information
   ok(cspReport["referrer"].startsWith("http://mochi.test:8888/tests/dom/security/test/csp/test_report.html"),
      "Incorrect referrer");
 
-  is(cspReport["blocked-uri"], "self", "Incorrect blocked-uri");
+  is(cspReport["blocked-uri"], "", "Incorrect blocked-uri");
 
   is(cspReport["violated-directive"], "default-src", "Incorrect violated-directive");
 
   is(cspReport["original-policy"], "default-src 'none'; report-uri http://mochi.test:8888/foo.sjs",
      "Incorrect original-policy");
 
   is(cspReport["source-file"], docUri, "Incorrect source-file");
 
--- a/dom/security/test/unit/test_csp_reports.js
+++ b/dom/security/test/unit/test_csp_reports.js
@@ -101,31 +101,31 @@ function run_test() {
   var selfuri = NetUtil.newURI(REPORT_SERVER_URI +
                                ":" + REPORT_SERVER_PORT +
                                "/foo/self");
 
   let content = Cc["@mozilla.org/supports-string;1"].
                    createInstance(Ci.nsISupportsString);
   content.data = "";
   // test that inline script violations cause a report.
-  makeTest(0, {"blocked-uri": "self"}, false,
+  makeTest(0, {"blocked-uri": ""}, false,
       function(csp) {
         let inlineOK = true;
         inlineOK = csp.getAllowsInline(Ci.nsIContentPolicy.TYPE_SCRIPT,
                                        "", // aNonce
                                        false, // aParserCreated
                                        content, // aContent
                                        0); // aLineNumber
 
         // this is not a report only policy, so it better block inline scripts
         Assert.ok(!inlineOK);
       });
 
   // test that eval violations cause a report.
-  makeTest(1, {"blocked-uri": "self",
+  makeTest(1, {"blocked-uri": "",
                // JSON script-sample is UTF8 encoded
                "script-sample" : "\xc2\xa3\xc2\xa5\xc2\xb5\xe5\x8c\x97\xf0\xa0\x9d\xb9"}, false,
       function(csp) {
         let evalOK = true, oReportViolation = {'value': false};
         evalOK = csp.getAllowsEval(oReportViolation);
 
         // this is not a report only policy, so it better block eval
         Assert.ok(!evalOK);
@@ -148,34 +148,34 @@ function run_test() {
       function(csp) {
         // shouldLoad creates and sends out the report here.
         csp.shouldLoad(Ci.nsIContentPolicy.TYPE_SCRIPT,
                       NetUtil.newURI("http://blocked.test/foo.js"),
                       null, null, null, null);
       });
 
   // test that inline script violations cause a report in report-only policy
-  makeTest(3, {"blocked-uri": "self"}, true,
+  makeTest(3, {"blocked-uri": ""}, true,
       function(csp) {
         let inlineOK = true;
         let content = Cc["@mozilla.org/supports-string;1"].
                          createInstance(Ci.nsISupportsString);
         content.data = "";
         inlineOK = csp.getAllowsInline(Ci.nsIContentPolicy.TYPE_SCRIPT,
                                        "", // aNonce
                                        false, // aParserCreated
                                        content, // aContent
                                        0); // aLineNumber
 
         // this is a report only policy, so it better allow inline scripts
         Assert.ok(inlineOK);
       });
 
   // test that eval violations cause a report in report-only policy
-  makeTest(4, {"blocked-uri": "self"}, true,
+  makeTest(4, {"blocked-uri": ""}, true,
       function(csp) {
         let evalOK = true, oReportViolation = {'value': false};
         evalOK = csp.getAllowsEval(oReportViolation);
 
         // this is a report only policy, so it better allow eval
         Assert.ok(evalOK);
         // ... but still cause reports to go out
         Assert.ok(oReportViolation.value);
--- a/toolkit/components/extensions/test/xpcshell/test_ext_contentscript_triggeringPrincipal.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_contentscript_triggeringPrincipal.js
@@ -830,17 +830,17 @@ function awaitCSP(urlsPromise) {
     let expectedURLs, blockedURLs, blockedSources;
     let queuedRequests = [];
 
     function checkRequest(request) {
       let body = JSON.parse(readUTF8InputStream(request.bodyInputStream));
       let report = body["csp-report"];
 
       let origURL = report["blocked-uri"];
-      if (origURL !== "self") {
+      if (origURL !== "self" && origURL !== "") {
         let {baseURL} = getOriginBase(origURL);
 
         if (expectedURLs.has(baseURL)) {
           ok(false, `Got unexpected CSP report for allowed URL ${origURL}`);
         }
 
         if (blockedURLs.has(baseURL)) {
           blockedURLs.delete(baseURL);