Bug 594446 - CSP report-uri should accept relative URIs [r=jst a=blocking2.0: beta7+]

content/base/src/CSPUtils.jsm

@@ -216,43 +216,65 @@ CSPRep.fromString = function(aStr, self) {
         }
       }
     }
-    
+
     // REPORT URI ///////////////////////////////////////////////////////
     if (dirname === UD.REPORT_URI) {
       // might be space-separated list of URIs
       var uriStrings = dirvalue.split(/\s+/);
       var okUriStrings = [];
 
-      // Verify that each report URI is in the same etld + 1
-      // if "self" is defined, and just that it's valid otherwise.
       for (let i in uriStrings) {
+        var uri = null;
         try {
-          var uri = gIoService.newURI(uriStrings[i],null,null);
+          // Relative URIs are okay, but to ensure we send the reports to the
+          // right spot, the relative URIs are expanded here during parsing.
+          // The resulting CSPRep instance will have only absolute URIs.
+          uri = gIoService.newURI(uriStrings[i],null,selfUri);
+
+          // if there's no host, don't do the ETLD+ check.  This will throw
+          // NS_ERROR_FAILURE if the URI doesn't have a host, causing a parse
+          // failure.
+          uri.host;
+
+          // Verify that each report URI is in the same etld + 1 and that the
+          // scheme and port match "self" if "self" is defined, and just that
+          // it's valid otherwise.
           if (self) {
-            if (gETLDService.getBaseDomain(uri) ===
+            if (gETLDService.getBaseDomain(uri) !==
                 gETLDService.getBaseDomain(selfUri)) {
-              okUriStrings.push(uriStrings[i]);
-            } else {
               CSPWarning("can't use report URI from non-matching eTLD+1: "
                          + gETLDService.getBaseDomain(uri));
+              continue;
+            }
+            if (!uri.schemeIs(selfUri.scheme)) {
+              CSPWarning("can't use report URI with different scheme from "
+                         + "originating document: " + uri.asciiSpec);
+              continue;
+            }
+            if (uri.port && uri.port !== selfUri.port) {
+              CSPWarning("can't use report URI with different port from "
+                         + "originating document: " + uri.asciiSpec);
+              continue;
             }
           }
         } catch(e) {
           switch (e.result) {
             case Components.results.NS_ERROR_INSUFFICIENT_DOMAIN_LEVELS:
             case Components.results.NS_ERROR_HOST_IS_IP_ADDRESS:
-              if (uri.host === selfUri.host) {
+              if (uri.host !== selfUri.host) {
-                okUriStrings.push(uriStrings[i]);
+                CSPWarning("page on " + selfUri.host
-              } else {
+                           + " cannot send reports to " + uri.host);
-                CSPWarning("page on " + selfUri.host + " cannot send reports to " + uri.host);
+                continue;
               }
               break;
 
             default:
               CSPWarning("couldn't parse report URI: " + uriStrings[i]);
-              break;
+              continue;
           }
         }
+        // all verification passed: same ETLD+1, scheme, and port.
+        okUriStrings.push(uri.asciiSpec);
       }
       aCSPR._directives[UD.REPORT_URI] = okUriStrings.join(' ');
       continue directive;

content/base/src/contentSecurityPolicy.js

@@ -271,6 +271,9 @@ ContentSecurityPolicy.prototype = {
                + (blockedUri['asciiSpec'] ? " by " + blockedUri.asciiSpec : ""));
 
       // For each URI in the report list, send out a report.
+      // We make the assumption that all of the URIs are absolute URIs; this
+      // should be taken care of in CSPRep.fromString (where it converts any
+      // relative URIs into absolute ones based on "self").
       for (let i in uris) {
         if (uris[i] === "")
           continue;

content/base/test/unit/test_csputils.js

@@ -46,6 +46,26 @@ const POLICY_PORT = 9000;
 const POLICY_URI = "http://localhost:" + POLICY_PORT + "/policy";
 const POLICY_URI_RELATIVE = "/policy";
 
+// helper to assert that an array has the given value somewhere.
+function do_check_in_array(arr, val, stack) {
+  if (!stack)
+    stack = Components.stack.caller;
+
+  var text = val + " in [" + arr.join(",") + "]";
+
+  for(var i in arr) {
+    dump(".......... " + i + "> " + arr[i] + "\n");
+    if(arr[i] == val) {
+      //succeed
+      ++_passedChecks;
+      dump("TEST-PASS | " + stack.filename + " | [" + stack.name + " : " +
+           stack.lineNumber + "] " + text + "\n");
+      return;
+    }
+  }
+  do_throw(text, stack);
+}
+
 // helper to assert that an object or array must have a given key
 function do_check_has_key(foo, key, stack) {
   if (!stack) 
@@ -493,6 +513,66 @@ test(function test_FrameAncestor_defaults() {
       do_check_false(cspr.permits("http://self.com", SD.FRAME_ANCESTORS));
       do_check_false(cspr.permits("http://subd.self.com:34", SD.FRAME_ANCESTORS));
      });
+
+test(function test_CSP_ReportURI_parsing() {
+      var cspr;
+      var SD = CSPRep.SRC_DIRECTIVES;
+      var self = "http://self.com:34";
+      var parsedURIs = [];
+
+      var uri_valid_absolute = self + "/report.py";
+      var uri_invalid_host_absolute = "http://foo.org:34/report.py";
+      var uri_valid_relative = "/report.py";
+      var uri_valid_relative_expanded = self + uri_valid_relative;
+      var uri_valid_relative2 = "foo/bar/report.py";
+      var uri_valid_relative2_expanded = self + "/" + uri_valid_relative2;
+      var uri_invalid_relative = "javascript:alert(1)";
+
+      cspr = CSPRep.fromString("allow *; report-uri " + uri_valid_absolute, self);
+      parsedURIs = cspr.getReportURIs().split(/\s+/);
+      do_check_in_array(parsedURIs, uri_valid_absolute);
+      do_check_eq(parsedURIs.length, 1);
+
+      cspr = CSPRep.fromString("allow *; report-uri " + uri_invalid_host_absolute, self);
+      parsedURIs = cspr.getReportURIs().split(/\s+/);
+      do_check_in_array(parsedURIs, "");
+      do_check_eq(parsedURIs.length, 1); // the empty string is in there.
+
+      cspr = CSPRep.fromString("allow *; report-uri " + uri_invalid_relative, self);
+      parsedURIs = cspr.getReportURIs().split(/\s+/);
+      do_check_in_array(parsedURIs, "");
+      do_check_eq(parsedURIs.length, 1);
+
+      cspr = CSPRep.fromString("allow *; report-uri " + uri_valid_relative, self);
+      parsedURIs = cspr.getReportURIs().split(/\s+/);
+      do_check_in_array(parsedURIs, uri_valid_relative_expanded);
+      do_check_eq(parsedURIs.length, 1);
+
+      cspr = CSPRep.fromString("allow *; report-uri " + uri_valid_relative2, self);
+      parsedURIs = cspr.getReportURIs().split(/\s+/);
+      dump(parsedURIs.length);
+      do_check_in_array(parsedURIs, uri_valid_relative2_expanded);
+      do_check_eq(parsedURIs.length, 1);
+
+      // combination!
+      cspr = CSPRep.fromString("allow *; report-uri " +
+                               uri_valid_relative2 + " " +
+                               uri_valid_absolute, self);
+      parsedURIs = cspr.getReportURIs().split(/\s+/);
+      do_check_in_array(parsedURIs, uri_valid_relative2_expanded);
+      do_check_in_array(parsedURIs, uri_valid_absolute);
+      do_check_eq(parsedURIs.length, 2);
+
+      cspr = CSPRep.fromString("allow *; report-uri " +
+                               uri_valid_relative2 + " " +
+                               uri_invalid_host_absolute + " " +
+                               uri_valid_absolute, self);
+      parsedURIs = cspr.getReportURIs().split(/\s+/);
+      do_check_in_array(parsedURIs, uri_valid_relative2_expanded);
+      do_check_in_array(parsedURIs, uri_valid_absolute);
+      do_check_eq(parsedURIs.length, 2);
+    });
+
 /*
 
 test(function test_CSPRep_fromPolicyURI_failswhenmixed() {