Commit 1b62e9e8 authored by Antonio Sartori's avatar Antonio Sartori Committed by Commit Bot

Fix wildcard host matching in CSPEE subsume algorithm

The previous implementation returned `true` for `*.example.com`
subsumes `example.com`. However, since `*.example.com` does not match
`example.com`, this should not be the case. And indeed according to
2.3.3 in
https://w3c.github.io/webappsec-cspee/#subsume-source-expressions in
this case the subsume algorithm should return `false`.

Bug: 1086857
Change-Id: I449f72d2db0a918478fc1ba4250335ae57a4ae2d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2210463Reviewed-by: default avatarMike West <mkwst@chromium.org>
Reviewed-by: default avatarArthur Sonzogni <arthursonzogni@chromium.org>
Commit-Queue: Antonio Sartori <antoniosartori@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805286}
parent 1d8f949f
...@@ -542,7 +542,7 @@ TEST_F(CSPDirectiveListTest, WorkerSrcChildSrcFallback) { ...@@ -542,7 +542,7 @@ TEST_F(CSPDirectiveListTest, WorkerSrcChildSrcFallback) {
TEST_F(CSPDirectiveListTest, SubsumesBasedOnCSPSourcesOnly) { TEST_F(CSPDirectiveListTest, SubsumesBasedOnCSPSourcesOnly) {
CSPDirectiveList* a = CreateList( CSPDirectiveList* a = CreateList(
"script-src http://*.one.com; img-src https://one.com " "script-src http://*.one.com; img-src https://sub.one.com "
"http://two.com/imgs/", "http://two.com/imgs/",
ContentSecurityPolicyType::kEnforce); ContentSecurityPolicyType::kEnforce);
...@@ -556,49 +556,53 @@ TEST_F(CSPDirectiveListTest, SubsumesBasedOnCSPSourcesOnly) { ...@@ -556,49 +556,53 @@ TEST_F(CSPDirectiveListTest, SubsumesBasedOnCSPSourcesOnly) {
{{"script-src http://example.com"}, false, false}, {{"script-src http://example.com"}, false, false},
{{"img-src http://example.com"}, false, false}, {{"img-src http://example.com"}, false, false},
{{"script-src http://*.one.com"}, false, true}, {{"script-src http://*.one.com"}, false, true},
{{"img-src https://one.com http://two.com/imgs/"}, false, true}, {{"img-src https://sub.one.com http://two.com/imgs/"}, false, true},
{{"default-src http://example.com"}, false, false}, {{"default-src http://example.com"}, false, false},
{{"default-src https://one.com http://two.com/imgs/"}, false, false}, {{"default-src https://sub.one.com http://two.com/imgs/"}, false, false},
{{"default-src http://one.com"}, false, false}, {{"default-src http://sub.one.com"}, false, false},
{{"script-src http://*.one.com; img-src http://two.com/"}, false, false}, {{"script-src http://*.one.com; img-src http://two.com/"}, false, false},
{{"script-src http://*.one.com", "img-src http://one.com"}, false, true}, {{"script-src http://*.one.com", "img-src http://sub.one.com"},
false,
true},
{{"script-src http://*.one.com", "script-src https://two.com"}, {{"script-src http://*.one.com", "script-src https://two.com"},
false, false,
true}, true},
{{"script-src http://*.random.com", "script-src https://random.com"}, {{"script-src http://*.random.com", "script-src https://random.com"},
false, false,
false}, false},
{{"script-src http://one.com", "script-src https://random.com"}, {{"script-src http://sub.one.com", "script-src https://random.com"},
false, false,
false}, false},
{{"script-src http://*.random.com; default-src http://one.com " {{"script-src http://*.random.com; default-src http://sub.one.com "
"http://two.com/imgs/", "http://two.com/imgs/",
"default-src https://random.com"}, "default-src https://sub.random.com"},
false, false,
false}, false},
// `listB`, which is as restrictive as `A`, is subsumed. // `listB`, which is as restrictive as `A`, is subsumed.
{{"default-src https://one.com"}, true, false}, {{"default-src https://sub.one.com"}, true, false},
{{"default-src http://random.com", {{"default-src http://random.com",
"default-src https://non-random.com:*"}, "default-src https://non-random.com:*"},
true, true,
false}, false},
{{"script-src http://*.one.com; img-src https://one.com"}, true, false}, {{"script-src http://*.one.com; img-src https://sub.one.com"},
{{"script-src http://*.one.com; img-src https://one.com " true,
false},
{{"script-src http://*.one.com; img-src https://sub.one.com "
"http://two.com/imgs/"}, "http://two.com/imgs/"},
true, true,
true}, true},
{{"script-src http://*.one.com", {{"script-src http://*.one.com",
"img-src https://one.com http://two.com/imgs/"}, "img-src https://sub.one.com http://two.com/imgs/"},
true, true,
true}, true},
{{"script-src http://*.random.com; default-src https://one.com " {{"script-src http://*.random.com; default-src https://sub.one.com "
"http://two.com/imgs/", "http://two.com/imgs/",
"default-src https://else.com"}, "default-src https://else.com"},
true, true,
false}, false},
{{"script-src http://*.random.com; default-src https://one.com " {{"script-src http://*.random.com; default-src https://sub.one.com "
"http://two.com/imgs/", "http://two.com/imgs/",
"default-src https://one.com"}, "default-src https://sub.one.com"},
true, true,
false}, false},
}; };
......
...@@ -203,7 +203,9 @@ bool CSPSource::Subsumes(CSPSource* other) const { ...@@ -203,7 +203,9 @@ bool CSPSource::Subsumes(CSPSource* other) const {
return false; return false;
} }
bool host_subsumes = (host_ == other->host_ || HostMatches(other->host_)); bool host_subsumes = other->host_wildcard_ == kHasWildcard
? HostMatches("." + other->host_)
: HostMatches(other->host_);
bool port_subsumes = (port_wildcard_ == kHasWildcard) || bool port_subsumes = (port_wildcard_ == kHasWildcard) ||
PortMatches(other->port_, other->scheme_) != PortMatches(other->port_, other->scheme_) !=
PortMatchingResult::kNotMatching; PortMatchingResult::kNotMatching;
...@@ -217,8 +219,9 @@ bool CSPSource::IsSimilar(CSPSource* other) const { ...@@ -217,8 +219,9 @@ bool CSPSource::IsSimilar(CSPSource* other) const {
other->SchemeMatches(scheme_) != SchemeMatchingResult::kNotMatching; other->SchemeMatches(scheme_) != SchemeMatchingResult::kNotMatching;
if (!schemes_match || IsSchemeOnly() || other->IsSchemeOnly()) if (!schemes_match || IsSchemeOnly() || other->IsSchemeOnly())
return schemes_match; return schemes_match;
bool hosts_match = (host_ == other->host_) || HostMatches(other->host_) || bool hosts_match =
other->HostMatches(host_); HostMatches((other->host_wildcard_ ? "*." : "") + other->host_) ||
other->HostMatches((host_wildcard_ ? "*." : "") + host_);
bool ports_match = bool ports_match =
(other->port_wildcard_ == kHasWildcard) || (other->port_wildcard_ == kHasWildcard) ||
PortMatches(other->port_, other->scheme_) != PortMatches(other->port_, other->scheme_) !=
......
...@@ -447,7 +447,7 @@ TEST_F(CSPSourceTest, WildcardsSubsumes) { ...@@ -447,7 +447,7 @@ TEST_F(CSPSourceTest, WildcardsSubsumes) {
true}, true},
{{CSPSource::kNoWildcard, CSPSource::kNoWildcard}, {{CSPSource::kNoWildcard, CSPSource::kNoWildcard},
{CSPSource::kHasWildcard, CSPSource::kNoWildcard}, {CSPSource::kHasWildcard, CSPSource::kNoWildcard},
true}, false},
// Two out of four possible wildcards. // Two out of four possible wildcards.
{{CSPSource::kHasWildcard, CSPSource::kHasWildcard}, {{CSPSource::kHasWildcard, CSPSource::kHasWildcard},
{CSPSource::kNoWildcard, CSPSource::kNoWildcard}, {CSPSource::kNoWildcard, CSPSource::kNoWildcard},
...@@ -466,7 +466,7 @@ TEST_F(CSPSourceTest, WildcardsSubsumes) { ...@@ -466,7 +466,7 @@ TEST_F(CSPSourceTest, WildcardsSubsumes) {
true}, true},
{{CSPSource::kNoWildcard, CSPSource::kNoWildcard}, {{CSPSource::kNoWildcard, CSPSource::kNoWildcard},
{CSPSource::kHasWildcard, CSPSource::kHasWildcard}, {CSPSource::kHasWildcard, CSPSource::kHasWildcard},
true}, false},
// Three out of four possible wildcards. // Three out of four possible wildcards.
{{CSPSource::kHasWildcard, CSPSource::kHasWildcard}, {{CSPSource::kHasWildcard, CSPSource::kHasWildcard},
{CSPSource::kHasWildcard, CSPSource::kNoWildcard}, {CSPSource::kHasWildcard, CSPSource::kNoWildcard},
...@@ -479,7 +479,7 @@ TEST_F(CSPSourceTest, WildcardsSubsumes) { ...@@ -479,7 +479,7 @@ TEST_F(CSPSourceTest, WildcardsSubsumes) {
true}, true},
{{CSPSource::kNoWildcard, CSPSource::kHasWildcard}, {{CSPSource::kNoWildcard, CSPSource::kHasWildcard},
{CSPSource::kHasWildcard, CSPSource::kHasWildcard}, {CSPSource::kHasWildcard, CSPSource::kHasWildcard},
true}, false},
// Four out of four possible wildcards. // Four out of four possible wildcards.
{{CSPSource::kHasWildcard, CSPSource::kHasWildcard}, {{CSPSource::kHasWildcard, CSPSource::kHasWildcard},
{CSPSource::kHasWildcard, CSPSource::kHasWildcard}, {CSPSource::kHasWildcard, CSPSource::kHasWildcard},
...@@ -717,16 +717,17 @@ TEST_F(CSPSourceTest, FirstSubsumesSecond) { ...@@ -717,16 +717,17 @@ TEST_F(CSPSourceTest, FirstSubsumesSecond) {
// If we add another source to `listB` with a host wildcard, // If we add another source to `listB` with a host wildcard,
// then the result should definitely be false. // then the result should definitely be false.
list_b.push_back(host_wildcard); list_b.push_back(host_wildcard);
EXPECT_FALSE(CSPSource::FirstSubsumesSecond(list_a, list_b));
// If we add another source to `listA` with a port wildcard, // If we add another source to `listA` with a port wildcard,
// it does not make `listB` to be subsumed under `listA`. // it does not make `listB` to be subsumed under `listA`.
list_b.push_back(port_wildcard); list_b.push_back(port_wildcard);
EXPECT_FALSE(CSPSource::FirstSubsumesSecond(list_a, list_b)); EXPECT_FALSE(CSPSource::FirstSubsumesSecond(list_a, list_b));
// If however we add another source to `listA` with both wildcards, // If however we add another source to `listA` with both wildcards, and the
// that CSPSource is subsumed, so the answer should be as expected // source with the port wildcard, the answer should be as expected before.
// before.
list_a.push_back(both_wildcards); list_a.push_back(both_wildcards);
list_a.push_back(port_wildcard);
EXPECT_EQ(test.expected, CSPSource::FirstSubsumesSecond(list_a, list_b)); EXPECT_EQ(test.expected, CSPSource::FirstSubsumesSecond(list_a, list_b));
// If we add a scheme-source expression of 'https' to `listB`, then it // If we add a scheme-source expression of 'https' to `listB`, then it
...@@ -762,21 +763,21 @@ TEST_F(CSPSourceTest, Intersect) { ...@@ -762,21 +763,21 @@ TEST_F(CSPSourceTest, Intersect) {
// Wildcards // Wildcards
{{"http", "example.com", "/", 0, CSPSource::kHasWildcard, {{"http", "example.com", "/", 0, CSPSource::kHasWildcard,
CSPSource::kNoWildcard}, CSPSource::kNoWildcard},
{"http", "example.com", "/", 0, CSPSource::kNoWildcard, {"http", "sub.example.com", "/", 0, CSPSource::kNoWildcard,
CSPSource::kNoWildcard}, CSPSource::kNoWildcard},
{"http", "example.com", "/", 0, CSPSource::kNoWildcard, {"http", "sub.example.com", "/", 0, CSPSource::kNoWildcard,
CSPSource::kNoWildcard}}, CSPSource::kNoWildcard}},
{{"http", "example.com", "/", 0, CSPSource::kHasWildcard, {{"http", "example.com", "/", 0, CSPSource::kHasWildcard,
CSPSource::kHasWildcard}, CSPSource::kHasWildcard},
{"http", "example.com", "/", 0, CSPSource::kNoWildcard, {"http", "sub.example.com", "/", 0, CSPSource::kNoWildcard,
CSPSource::kNoWildcard}, CSPSource::kNoWildcard},
{"http", "example.com", "/", 0, CSPSource::kNoWildcard, {"http", "sub.example.com", "/", 0, CSPSource::kNoWildcard,
CSPSource::kNoWildcard}}, CSPSource::kNoWildcard}},
{{"http", "example.com", "/", 0, CSPSource::kHasWildcard, {{"http", "example.com", "/", 0, CSPSource::kHasWildcard,
CSPSource::kNoWildcard}, CSPSource::kNoWildcard},
{"http", "example.com", "/", 0, CSPSource::kNoWildcard, {"http", "sub.example.com", "/", 0, CSPSource::kNoWildcard,
CSPSource::kHasWildcard}, CSPSource::kHasWildcard},
{"http", "example.com", "/", 0, CSPSource::kNoWildcard, {"http", "sub.example.com", "/", 0, CSPSource::kNoWildcard,
CSPSource::kNoWildcard}}, CSPSource::kNoWildcard}},
// Ports // Ports
{{"http", "example.com", "/", 80, CSPSource::kNoWildcard, {{"http", "example.com", "/", 80, CSPSource::kNoWildcard,
......
...@@ -229,36 +229,36 @@ TEST_F(SourceListDirectiveTest, GetIntersectCSPSources) { ...@@ -229,36 +229,36 @@ TEST_F(SourceListDirectiveTest, GetIntersectCSPSources) {
String sources; String sources;
String expected; String expected;
} cases[] = { } cases[] = {
{"http://example1.com/foo/ http://example2.com/bar/", {"http://example1.com/foo/ http://sub.example2.com/bar/",
"http://example1.com/foo/ http://example2.com/bar/"}, "http://example1.com/foo/ http://sub.example2.com/bar/"},
// Normalizing schemes. // Normalizing schemes.
{"https://example1.com/foo/ http://example2.com/bar/", {"https://example1.com/foo/ http://sub.example2.com/bar/",
"https://example1.com/foo/ http://example2.com/bar/"}, "https://example1.com/foo/ http://sub.example2.com/bar/"},
{"https://example1.com/foo/ https://example2.com/bar/", {"https://example1.com/foo/ https://sub.example2.com/bar/",
"https://example1.com/foo/ https://example2.com/bar/"}, "https://example1.com/foo/ https://sub.example2.com/bar/"},
{"https://example1.com/foo/ wss://example2.com/bar/", {"https://example1.com/foo/ wss://sub.example2.com/bar/",
"https://example1.com/foo/"}, "https://example1.com/foo/"},
// Normalizing hosts. // Normalizing hosts.
{"http://*.example1.com/foo/ http://*.example2.com/bar/", {"http://*.com/foo/ http://*.example2.com/bar/",
"http://example1.com/foo/ http://*.example2.com/bar/"}, "http://example1.com/foo/ http://*.example2.com/bar/"},
{"http://*.example1.com/foo/ http://foo.example2.com/bar/", {"http://*.com/foo/ http://foo.example2.com/bar/",
"http://example1.com/foo/ http://foo.example2.com/bar/"}, "http://example1.com/foo/ http://foo.example2.com/bar/"},
// Normalizing ports. // Normalizing ports.
{"http://example1.com/foo/ http://example2.com/bar/", {"http://example1.com/foo/ http://sub.example2.com/bar/",
"http://example1.com/foo/ http://example2.com/bar/"}, "http://example1.com/foo/ http://sub.example2.com/bar/"},
{"http://example1.com/foo/ http://example2.com:90/bar/", {"http://example1.com/foo/ http://sub.example2.com:90/bar/",
"http://example1.com/foo/"}, "http://example1.com/foo/"},
{"http://example1.com:*/foo/ http://example2.com/bar/", {"http://example1.com:*/foo/ http://sub.example2.com/bar/",
"http://example1.com/foo/ http://example2.com/bar/"}, "http://example1.com/foo/ http://sub.example2.com/bar/"},
{"http://*.example3.com:100/bar/ http://example1.com/foo/", {"http://*.example3.com:100/bar/ http://example1.com/foo/",
"http://example1.com/foo/ http://*.example3.com:100/bar/"}, "http://example1.com/foo/ http://*.example3.com:100/bar/"},
// Normalizing paths. // Normalizing paths.
{"http://example1.com/ http://example2.com/", {"http://example1.com/ http://sub.example2.com/",
"http://example1.com/foo/ http://example2.com/bar/"}, "http://example1.com/foo/ http://sub.example2.com/bar/"},
{"http://example1.com/foo/index.html http://example2.com/bar/", {"http://example1.com/foo/index.html http://sub.example2.com/bar/",
"http://example1.com/foo/index.html http://example2.com/bar/"}, "http://example1.com/foo/index.html http://sub.example2.com/bar/"},
{"http://example1.com/bar http://example2.com/bar/", {"http://example1.com/bar http://sub.example2.com/bar/",
"http://example2.com/bar/"}, "http://sub.example2.com/bar/"},
// Not similar to be normalized // Not similar to be normalized
{"http://non-example1.com/foo/ http://non-example2.com/bar/", ""}, {"http://non-example1.com/foo/ http://non-example2.com/bar/", ""},
{"https://non-example1.com/foo/ wss://non-example2.com/bar/", ""}, {"https://non-example1.com/foo/ wss://non-example2.com/bar/", ""},
...@@ -303,8 +303,8 @@ TEST_F(SourceListDirectiveTest, GetIntersectCSPSourcesSchemes) { ...@@ -303,8 +303,8 @@ TEST_F(SourceListDirectiveTest, GetIntersectCSPSourcesSchemes) {
{"https: http: wss:", "http: wss:"}, {"https: http: wss:", "http: wss:"},
{"https: http://another-example1.com/bar/", {"https: http://another-example1.com/bar/",
"https: http://another-example1.com/bar/"}, "https: http://another-example1.com/bar/"},
{"http://*.example1.com/", {"http://*.com/",
"http://*.example1.com/ http://example1.com/foo/ " "http://*.com/ http://example1.com/foo/ "
"https://example1.com/foo/ http://example1.com/bar/page.html"}, "https://example1.com/foo/ http://example1.com/bar/page.html"},
{"http://example1.com/foo/ https://example1.com/foo/", {"http://example1.com/foo/ https://example1.com/foo/",
"http://example1.com/foo/ https://example1.com/foo/ " "http://example1.com/foo/ https://example1.com/foo/ "
...@@ -364,7 +364,7 @@ TEST_F(SourceListDirectiveTest, Subsumes) { ...@@ -364,7 +364,7 @@ TEST_F(SourceListDirectiveTest, Subsumes) {
{{"https://example1.com/foo/", {{"https://example1.com/foo/",
"http://*.example1.com/foo/ http://*.example2.com/bar/"}, "http://*.example1.com/foo/ http://*.example2.com/bar/"},
true}, true},
{{"http://example2.com/bar/", {{"http://sub.example2.com/bar/",
"http://*.example3.com:*/bar/ http://*.example2.com/bar/"}, "http://*.example3.com:*/bar/ http://*.example2.com/bar/"},
true}, true},
{{"http://example3.com:100/bar/", {{"http://example3.com:100/bar/",
...@@ -372,12 +372,12 @@ TEST_F(SourceListDirectiveTest, Subsumes) { ...@@ -372,12 +372,12 @@ TEST_F(SourceListDirectiveTest, Subsumes) {
true}, true},
// Lists that intersect into two of the required sources are subsumed. // Lists that intersect into two of the required sources are subsumed.
{{"http://example1.com/foo/ http://*.example2.com/bar/"}, true}, {{"http://example1.com/foo/ http://*.example2.com/bar/"}, true},
{{"http://example1.com/foo/ http://example2.com/bar/", {{"http://example1.com/foo/ http://sub.example2.com/bar/",
"http://example2.com/bar/ http://example1.com/foo/"}, "http://sub.example2.com/bar/ http://example1.com/foo/"},
true}, true},
// Ordering should not matter. // Ordering should not matter.
{{"https://example1.com/foo/ https://example2.com/bar/", {{"https://example1.com/foo/ https://sub.example2.com/bar/",
"http://example2.com/bar/ http://example1.com/foo/"}, "http://sub.example2.com/bar/ http://example1.com/foo/"},
true}, true},
// Lists that intersect into a policy identical to the required list are // Lists that intersect into a policy identical to the required list are
// subsumed. // subsumed.
...@@ -402,12 +402,12 @@ TEST_F(SourceListDirectiveTest, Subsumes) { ...@@ -402,12 +402,12 @@ TEST_F(SourceListDirectiveTest, Subsumes) {
{{"http://example1.com/foo/ http://*.example2.com/bar/ " {{"http://example1.com/foo/ http://*.example2.com/bar/ "
"http://*.example3.com:*/bar/ http://*.example4.com:*/bar/"}, "http://*.example3.com:*/bar/ http://*.example4.com:*/bar/"},
false}, false},
{{"http://example1.com/foo/ http://example2.com/foo/"}, false}, {{"http://example1.com/foo/ http://sub.example2.com/foo/"}, false},
{{"http://*.example1.com/bar/", "http://example1.com/bar/"}, false}, {{"http://*.com/bar/", "http://example1.com/bar/"}, false},
{{"http://*.example1.com/foo/"}, false}, {{"http://*.example1.com/foo/"}, false},
{{"wss://example2.com/bar/"}, false}, {{"wss://sub.example2.com/bar/"}, false},
{{"http://*.non-example3.com:*/bar/"}, false}, {{"http://*.non-example3.com:*/bar/"}, false},
{{"http://example3.com/foo/"}, false}, {{"http://sub.example3.com/foo/"}, false},
{{"http://not-example1.com", "http://not-example1.com"}, false}, {{"http://not-example1.com", "http://not-example1.com"}, false},
{{"http://*", "http://*.com http://*.example3.com:*/bar/"}, false}, {{"http://*", "http://*.com http://*.example3.com:*/bar/"}, false},
}; };
......
...@@ -41,6 +41,15 @@ ...@@ -41,6 +41,15 @@
"required_csp": "frame-src http://c.com:443 http://b.com", "required_csp": "frame-src http://c.com:443 http://b.com",
"returned_csp": "frame-src http://b.com:80 http://c.com:443", "returned_csp": "frame-src http://b.com:80 http://c.com:443",
"expected": IframeLoad.EXPECT_LOAD }, "expected": IframeLoad.EXPECT_LOAD },
{ "name": "Host wildcard *.a.com does not match a.com",
"required_csp": "frame-src http://*.a.com",
"returned_csp": "frame-src http://a.com",
"expected": IframeLoad.EXPECT_BLOCK },
{ "name": "Host intersection with wildcards is computed correctly.",
"required_csp": "frame-sr 'none'",
"returned_csp": "frame-src http://a.com",
"returned_csp_2": "frame-src http://*.a.com",
"expected": IframeLoad.EXPECT_LOAD },
{ "name": "Iframe should load even if the ports are different but are default for the protocols.", { "name": "Iframe should load even if the ports are different but are default for the protocols.",
"required_csp": "frame-src http://b.com:80", "required_csp": "frame-src http://b.com:80",
"returned_csp": "child-src https://b.com:443", "returned_csp": "child-src https://b.com:443",
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment