Commit 14fd659a authored by rdsmith's avatar rdsmith Committed by Commit bot

Fix dictionary domain check problem.

Previous to this change, suffixing a "." to the URL from which a
dictionary was being suggested would allow servers to bypass the
check against a subdomain (e.g. evil.protected.good.com) setting a
dictionary to be used by a superdomain (.good.com).

BUG=389451
R=rsleevi@chromium.org

Review URL: https://codereview.chromium.org/574283006

Cr-Commit-Position: refs/heads/master@{#296246}
parent 67a11888
......@@ -13,6 +13,27 @@
#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
#include "net/url_request/url_request_http_job.h"
namespace {
void StripTrailingDot(GURL* gurl) {
std::string host(gurl->host());
if (host.empty())
return;
if (*host.rbegin() != '.')
return;
host.resize(host.size() - 1);
GURL::Replacements replacements;
replacements.SetHostStr(host);
*gurl = gurl->ReplaceComponents(replacements);
return;
}
} // namespace
namespace net {
//------------------------------------------------------------------------------
......@@ -550,10 +571,14 @@ void SdchManager::AddSdchDictionary(const std::string& dictionary_text,
line_start = line_end + 1;
}
if (!IsInSupportedDomain(dictionary_url))
// Narrow fix for http://crbug.com/389451.
GURL dictionary_url_normalized(dictionary_url);
StripTrailingDot(&dictionary_url_normalized);
if (!IsInSupportedDomain(dictionary_url_normalized))
return;
if (!Dictionary::CanSet(domain, path, ports, dictionary_url))
if (!Dictionary::CanSet(domain, path, ports, dictionary_url_normalized))
return;
// TODO(jar): Remove these hacks to preclude a DOS attack involving piles of
......@@ -574,7 +599,8 @@ void SdchManager::AddSdchDictionary(const std::string& dictionary_text,
<< " and server hash " << server_hash;
Dictionary* dictionary =
new Dictionary(dictionary_text, header_end + 2, client_hash,
dictionary_url, domain, path, expiration, ports);
dictionary_url_normalized, domain,
path, expiration, ports);
dictionaries_[server_hash] = dictionary;
return;
}
......
......@@ -278,6 +278,15 @@ TEST_F(SdchManagerTest, FailToSetDotHostPrefixDomainDictionary) {
GURL("http://w.x.y.z.google.com")));
}
TEST_F(SdchManagerTest, FailToSetDotHostPrefixDomainDictionaryTrailingDot) {
std::string dictionary_domain("x.y.z.google.com");
std::string dictionary_text(NewSdchDictionary(dictionary_domain));
// Fail the HD with D being the domain and H having a dot requirement.
EXPECT_FALSE(AddSdchDictionary(dictionary_text,
GURL("http://w.x.y.z.google.com.")));
}
TEST_F(SdchManagerTest, FailToSetRepeatPrefixWithDotDictionary) {
// Make sure that a prefix that matches the domain postfix won't confuse
// the validation checks.
......@@ -300,6 +309,42 @@ TEST_F(SdchManagerTest, CanSetLeadingDotDomainDictionary) {
EXPECT_TRUE(AddSdchDictionary(dictionary_text, GURL("http://www.google.com")));
}
TEST_F(SdchManagerTest,
CanSetLeadingDotDomainDictionaryFromURLWithTrailingDot) {
// Make sure that a prefix that matches the domain postfix won't confuse
// the validation checks.
std::string dictionary_domain(".google.com");
std::string dictionary_text(NewSdchDictionary(dictionary_domain));
// Verify that a leading dot in the domain is acceptable, as long as the host
// name does not contain any dots preceding the matched domain name.
EXPECT_TRUE(AddSdchDictionary(dictionary_text,
GURL("http://www.google.com.")));
}
TEST_F(SdchManagerTest, CannotSetLeadingDotDomainDictionary) {
// Make sure that a prefix that matches the domain postfix won't confuse
// the validation checks.
std::string dictionary_domain(".google.com");
std::string dictionary_text(NewSdchDictionary(dictionary_domain));
// Verify that a leading dot in the domain does not affect the name containing
// dots failure.
EXPECT_FALSE(AddSdchDictionary(dictionary_text,
GURL("http://www.subdomain.google.com")));
}
TEST_F(SdchManagerTest, CannotSetLeadingDotDomainDictionaryTrailingDot) {
// Make sure that a prefix that matches the domain postfix won't confuse
// the validation checks.
std::string dictionary_domain(".google.com");
std::string dictionary_text(NewSdchDictionary(dictionary_domain));
// Verify that a trailing period in the URL doesn't affect the check.
EXPECT_FALSE(AddSdchDictionary(dictionary_text,
GURL("http://www.subdomain.google.com.")));
}
// Make sure the order of the tests is not helping us or confusing things.
// See test CanSetExactMatchDictionary above for first try.
TEST_F(SdchManagerTest, CanStillSetExactMatchDictionary) {
......@@ -509,4 +554,3 @@ TEST_F(SdchManagerTest, ClearDictionaryData) {
}
} // namespace net
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