Bug 843311 - update CSP report-uri parsing to be spec compliant. r=grobinson

This commit is contained in:
Sid Stamm 2014-01-24 10:24:08 -08:00
parent 580a831cd3
commit e7799ef414
3 changed files with 52 additions and 71 deletions

View File

@ -386,32 +386,17 @@ CSPRep.fromString = function(aStr, self, reportOnly, docRequest, csp) {
// 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.
// if there's no host, this will throw NS_ERROR_FAILURE, 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) !==
gETLDService.getBaseDomain(selfUri)) {
cspWarn(aCSPR,
CSPLocalizer.getFormatStr("reportURInotETLDPlus1",
[gETLDService.getBaseDomain(uri)]));
continue;
}
if (!uri.schemeIs(selfUri.scheme)) {
cspWarn(aCSPR, CSPLocalizer.getFormatStr("reportURInotSameSchemeAsSelf",
[uri.asciiSpec]));
continue;
}
if (uri.port && uri.port !== selfUri.port) {
cspWarn(aCSPR, CSPLocalizer.getFormatStr("reportURInotSamePortAsSelf",
[uri.asciiSpec]));
continue;
}
// warn about, but do not prohibit non-http and non-https schemes for
// reporting URIs. The spec allows unrestricted URIs resolved
// relative to "self", but we should let devs know if the scheme is
// abnormal and may fail a POST.
if (!uri.schemeIs("http") && !uri.schemeIs("https")) {
cspWarn(aCSPR, CSPLocalizer.getFormatStr("reportURInotHttpsOrHttp",
[uri.asciiSpec]));
}
} catch(e) {
switch (e.result) {
@ -431,7 +416,7 @@ CSPRep.fromString = function(aStr, self, reportOnly, docRequest, csp) {
continue;
}
}
// all verification passed: same ETLD+1, scheme, and port.
// all verification passed
okUriStrings.push(uri.asciiSpec);
}
aCSPR._directives[UD.REPORT_URI] = okUriStrings.join(' ');
@ -649,34 +634,17 @@ CSPRep.fromStringSpecCompliant = function(aStr, self, reportOnly, docRequest, cs
// 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.
// if there's no host, this will throw NS_ERROR_FAILURE, 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) !==
gETLDService.getBaseDomain(selfUri)) {
cspWarn(aCSPR,
CSPLocalizer.getFormatStr("reportURInotETLDPlus1",
[gETLDService.getBaseDomain(uri)]));
continue;
}
if (!uri.schemeIs(selfUri.scheme)) {
cspWarn(aCSPR,
CSPLocalizer.getFormatStr("reportURInotSameSchemeAsSelf",
[uri.asciiSpec]));
continue;
}
if (uri.port && uri.port !== selfUri.port) {
cspWarn(aCSPR,
CSPLocalizer.getFormatStr("reportURInotSamePortAsSelf",
[uri.asciiSpec]));
continue;
}
// warn about, but do not prohibit non-http and non-https schemes for
// reporting URIs. The spec allows unrestricted URIs resolved
// relative to "self", but we should let devs know if the scheme is
// abnormal and may fail a POST.
if (!uri.schemeIs("http") && !uri.schemeIs("https")) {
cspWarn(aCSPR, CSPLocalizer.getFormatStr("reportURInotHttpsOrHttp",
[uri.asciiSpec]));
}
} catch(e) {
switch (e.result) {
@ -695,7 +663,7 @@ CSPRep.fromStringSpecCompliant = function(aStr, self, reportOnly, docRequest, cs
continue;
}
}
// all verification passed: same ETLD+1, scheme, and port.
// all verification passed.
okUriStrings.push(uri.asciiSpec);
}
aCSPR._directives[UD.REPORT_URI] = okUriStrings.join(' ');

View File

@ -665,46 +665,62 @@ test(function test_FrameAncestor_ignores_userpass_bug779918() {
test(function test_CSP_ReportURI_parsing() {
var cspr;
var SD = CSPRep.SRC_DIRECTIVES_OLD;
var SD = CSPRep.SRC_DIRECTIVES_NEW;
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_other_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)";
var uri_other_scheme_absolute = "https://self.com/report.py";
var uri_other_scheme_and_host_absolute = "https://foo.com/report.py";
cspr = CSPRep.fromString("allow *; report-uri " + uri_valid_absolute, URI(self));
cspr = CSPRep.fromStringSpecCompliant("default-src *; report-uri " + uri_valid_absolute, URI(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, URI(self));
cspr = CSPRep.fromStringSpecCompliant("default-src *; report-uri " + uri_other_host_absolute, URI(self));
parsedURIs = cspr.getReportURIs().split(/\s+/);
do_check_in_array(parsedURIs, "");
do_check_in_array(parsedURIs, uri_other_host_absolute);
do_check_eq(parsedURIs.length, 1); // the empty string is in there.
cspr = CSPRep.fromString("allow *; report-uri " + uri_invalid_relative, URI(self));
cspr = CSPRep.fromStringSpecCompliant("default-src *; report-uri " + uri_invalid_relative, URI(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, URI(self));
cspr = CSPRep.fromStringSpecCompliant("default-src *; report-uri " + uri_valid_relative, URI(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, URI(self));
cspr = CSPRep.fromStringSpecCompliant("default-src *; report-uri " + uri_valid_relative2, URI(self));
parsedURIs = cspr.getReportURIs().split(/\s+/);
dump(parsedURIs.length);
do_check_in_array(parsedURIs, uri_valid_relative2_expanded);
do_check_eq(parsedURIs.length, 1);
// make sure cross-scheme reporting works
cspr = CSPRep.fromStringSpecCompliant("default-src *; report-uri " + uri_other_scheme_absolute, URI(self));
parsedURIs = cspr.getReportURIs().split(/\s+/);
dump(parsedURIs.length);
do_check_in_array(parsedURIs, uri_other_scheme_absolute);
do_check_eq(parsedURIs.length, 1);
// make sure cross-scheme, cross-host reporting works
cspr = CSPRep.fromStringSpecCompliant("default-src *; report-uri " + uri_other_scheme_and_host_absolute, URI(self));
parsedURIs = cspr.getReportURIs().split(/\s+/);
dump(parsedURIs.length);
do_check_in_array(parsedURIs, uri_other_scheme_and_host_absolute);
do_check_eq(parsedURIs.length, 1);
// combination!
cspr = CSPRep.fromString("allow *; report-uri " +
cspr = CSPRep.fromStringSpecCompliant("default-src *; report-uri " +
uri_valid_relative2 + " " +
uri_valid_absolute, URI(self));
parsedURIs = cspr.getReportURIs().split(/\s+/);
@ -712,14 +728,15 @@ test(function test_CSP_ReportURI_parsing() {
do_check_in_array(parsedURIs, uri_valid_absolute);
do_check_eq(parsedURIs.length, 2);
cspr = CSPRep.fromString("allow *; report-uri " +
cspr = CSPRep.fromStringSpecCompliant("default-src *; report-uri " +
uri_valid_relative2 + " " +
uri_invalid_host_absolute + " " +
uri_other_host_absolute + " " +
uri_valid_absolute, URI(self));
parsedURIs = cspr.getReportURIs().split(/\s+/);
do_check_in_array(parsedURIs, uri_valid_relative2_expanded);
do_check_in_array(parsedURIs, uri_other_host_absolute);
do_check_in_array(parsedURIs, uri_valid_absolute);
do_check_eq(parsedURIs.length, 2);
do_check_eq(parsedURIs.length, 3);
});
test(

View File

@ -25,13 +25,9 @@ couldNotProcessUnknownDirective = Couldn't process unknown directive '%1$S'
# LOCALIZATION NOTE (ignoringUnknownOption):
# %1$S is the option that could not be understood
ignoringUnknownOption = Ignoring unknown option %1$S
# LOCALIZATION NOTE (reportURInotETLDPlus1):
# %1$S is the ETLD of the report URI that could not be used
reportURInotETLDPlus1 = The report URI (%1$S) must be from the same eTLD+1.
# LOCALIZATION NOTE (reportURInotSameSchemeAsSelf, reportURInotSamePortAsSelf):
# %1$S is the report URI that could not be used
reportURInotSameSchemeAsSelf = The report URI (%1$S) must use the same scheme as the originating document.
reportURInotSamePortAsSelf = The report URI (%1$S) must use the same port as the originating document.
# LOCALIZATION NOTE (reportURInotHttpsOrHttp):
# %1$S is the ETLD of the report URI that is not HTTP or HTTPS
reportURInotHttpsOrHttp = The report URI (%1$) should be an HTTP or HTTPS URI.
# LOCALIZATION NOTE (pageCannotSendReportsTo):
# %1$S is the URI of the page with the policy
# %2$S is the report URI that could not be used