Bug 1236222 - CSP: Blocked URI should be empty for inline violations. r?ckerschb draft
authorJonathan Kingston <jkt@mozilla.com>
Thu, 08 Mar 2018 16:23:03 -0800
changeset 777246 84b4aafd0be30b51dd5be5060f537b705c0742a7
parent 777163 ff0efa4132f0efd78af0910762aec7dcc1a8de66
push id105130
push userbmo:jkt@mozilla.com
push dateWed, 04 Apr 2018 13:47:51 +0000
reviewersckerschb
bugs1236222
milestone61.0a1
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);