Avoid plausible DOS attack by malicious SDCH server

Restrict SDCH to ONLY work with HTTP (fail safe security policy for this
experimental protocol).

Also add a histogram to see how often we encounter dictionary corruption
(which will evidence itself by having a multitude of dicitionaries
adverttised, with no real use server side).

r=ajenjo,kmixter

Review URL: http://codereview.chromium.org/11209

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@5628 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 272 insertions, 5 deletions

diff --git a/net/base/sdch_filter_unittest.cc b/net/base/sdch_filter_unittest.cc
index 81fc3e9..9ed9710 100644
--- a/net/base/sdch_filter_unittest.cc
+++ b/net/base/sdch_filter_unittest.cc
@@ -250,6 +250,166 @@ TEST_F(SdchFilterTest, BasicDictionary) {
   EXPECT_EQ(output, expanded_);
 }
 
+TEST_F(SdchFilterTest, NoDecodeHttps) {
+  // Construct a valid SDCH dictionary from a VCDIFF dictionary.
+  const std::string kSampleDomain = "sdchtest.com";
+  std::string dictionary(NewSdchDictionary(kSampleDomain));
+
+  std::string url_string = "http://" + kSampleDomain;
+
+  GURL url(url_string);
+  EXPECT_TRUE(sdch_manager_->AddSdchDictionary(dictionary, url));
+
+  std::string compressed(NewSdchCompressedData(dictionary));
+
+  std::vector<Filter::FilterType> filter_types;
+  filter_types.push_back(Filter::FILTER_TYPE_SDCH);
+
+  const int kInputBufferSize(100);
+  scoped_ptr<Filter> filter(Filter::Factory(filter_types, kInputBufferSize));
+  const size_t feed_block_size(100);
+  const size_t output_block_size(100);
+  std::string output;
+
+  filter->SetURL(GURL("https://" + kSampleDomain));
+  EXPECT_FALSE(FilterTestData(compressed, feed_block_size, output_block_size,
+                             filter.get(), &output));
+}
+
+// Current failsafe TODO/hack refuses to decode any content that doesn't use
+// http as the scheme (see use of DICTIONARY_SELECTED_FOR_NON_HTTP).
+// The following tests this blockage.  Note that blacklisting results, so we
+// we need separate tests for each of these.
+TEST_F(SdchFilterTest, NoDecodeFtp) {
+  // Construct a valid SDCH dictionary from a VCDIFF dictionary.
+  const std::string kSampleDomain = "sdchtest.com";
+  std::string dictionary(NewSdchDictionary(kSampleDomain));
+
+  std::string url_string = "http://" + kSampleDomain;
+
+  GURL url(url_string);
+  EXPECT_TRUE(sdch_manager_->AddSdchDictionary(dictionary, url));
+
+  std::string compressed(NewSdchCompressedData(dictionary));
+
+  std::vector<Filter::FilterType> filter_types;
+  filter_types.push_back(Filter::FILTER_TYPE_SDCH);
+
+  const int kInputBufferSize(100);
+  scoped_ptr<Filter> filter(Filter::Factory(filter_types, kInputBufferSize));
+  const size_t feed_block_size(100);
+  const size_t output_block_size(100);
+  std::string output;
+
+  filter->SetURL(GURL("ftp://" + kSampleDomain));
+  EXPECT_FALSE(FilterTestData(compressed, feed_block_size, output_block_size,
+                             filter.get(), &output));
+}
+
+TEST_F(SdchFilterTest, NoDecodeFileColon) {
+  // Construct a valid SDCH dictionary from a VCDIFF dictionary.
+  const std::string kSampleDomain = "sdchtest.com";
+  std::string dictionary(NewSdchDictionary(kSampleDomain));
+
+  std::string url_string = "http://" + kSampleDomain;
+
+  GURL url(url_string);
+  EXPECT_TRUE(sdch_manager_->AddSdchDictionary(dictionary, url));
+
+  std::string compressed(NewSdchCompressedData(dictionary));
+
+  std::vector<Filter::FilterType> filter_types;
+  filter_types.push_back(Filter::FILTER_TYPE_SDCH);
+
+  const int kInputBufferSize(100);
+  scoped_ptr<Filter> filter(Filter::Factory(filter_types, kInputBufferSize));
+  const size_t feed_block_size(100);
+  const size_t output_block_size(100);
+  std::string output;
+
+  filter->SetURL(GURL("file://" + kSampleDomain));
+  EXPECT_FALSE(FilterTestData(compressed, feed_block_size, output_block_size,
+                             filter.get(), &output));
+}
+
+TEST_F(SdchFilterTest, NoDecodeAboutColon) {
+  // Construct a valid SDCH dictionary from a VCDIFF dictionary.
+  const std::string kSampleDomain = "sdchtest.com";
+  std::string dictionary(NewSdchDictionary(kSampleDomain));
+
+  std::string url_string = "http://" + kSampleDomain;
+
+  GURL url(url_string);
+  EXPECT_TRUE(sdch_manager_->AddSdchDictionary(dictionary, url));
+
+  std::string compressed(NewSdchCompressedData(dictionary));
+
+  std::vector<Filter::FilterType> filter_types;
+  filter_types.push_back(Filter::FILTER_TYPE_SDCH);
+
+  const int kInputBufferSize(100);
+  scoped_ptr<Filter> filter(Filter::Factory(filter_types, kInputBufferSize));
+  const size_t feed_block_size(100);
+  const size_t output_block_size(100);
+  std::string output;
+
+  filter->SetURL(GURL("about://" + kSampleDomain));
+  EXPECT_FALSE(FilterTestData(compressed, feed_block_size, output_block_size,
+                             filter.get(), &output));
+}
+
+TEST_F(SdchFilterTest, NoDecodeJavaScript) {
+  // Construct a valid SDCH dictionary from a VCDIFF dictionary.
+  const std::string kSampleDomain = "sdchtest.com";
+  std::string dictionary(NewSdchDictionary(kSampleDomain));
+
+  std::string url_string = "http://" + kSampleDomain;
+
+  GURL url(url_string);
+  EXPECT_TRUE(sdch_manager_->AddSdchDictionary(dictionary, url));
+
+  std::string compressed(NewSdchCompressedData(dictionary));
+
+  std::vector<Filter::FilterType> filter_types;
+  filter_types.push_back(Filter::FILTER_TYPE_SDCH);
+
+  const int kInputBufferSize(100);
+  scoped_ptr<Filter> filter(Filter::Factory(filter_types, kInputBufferSize));
+  const size_t feed_block_size(100);
+  const size_t output_block_size(100);
+  std::string output;
+
+  filter->SetURL(GURL("javascript://" + kSampleDomain));
+  EXPECT_FALSE(FilterTestData(compressed, feed_block_size, output_block_size,
+                             filter.get(), &output));
+}
+
+TEST_F(SdchFilterTest, CanStillDecodeHttp) {
+  // Construct a valid SDCH dictionary from a VCDIFF dictionary.
+  const std::string kSampleDomain = "sdchtest.com";
+  std::string dictionary(NewSdchDictionary(kSampleDomain));
+
+  std::string url_string = "http://" + kSampleDomain;
+
+  GURL url(url_string);
+  EXPECT_TRUE(sdch_manager_->AddSdchDictionary(dictionary, url));
+
+  std::string compressed(NewSdchCompressedData(dictionary));
+
+  std::vector<Filter::FilterType> filter_types;
+  filter_types.push_back(Filter::FILTER_TYPE_SDCH);
+
+  const int kInputBufferSize(100);
+  scoped_ptr<Filter> filter(Filter::Factory(filter_types, kInputBufferSize));
+  const size_t feed_block_size(100);
+  const size_t output_block_size(100);
+  std::string output;
+
+  filter->SetURL(GURL("http://" + kSampleDomain));
+  EXPECT_TRUE(FilterTestData(compressed, feed_block_size, output_block_size,
+                             filter.get(), &output));
+}
+
 TEST_F(SdchFilterTest, CrossDomainDictionaryUse) {
   // Construct a valid SDCH dictionary from a VCDIFF dictionary.
   const std::string kSampleDomain = "sdchtest.com";
@@ -627,7 +787,8 @@ TEST_F(SdchFilterTest, CanSetLeadingDotDomainDictionary) {
   std::string dictionary_domain(".google.com");
   std::string dictionary_text(NewSdchDictionary(dictionary_domain));
 
-  // Fail the HD with D being the domain and H having a dot requirement.
+  // 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(sdch_manager_->AddSdchDictionary(dictionary_text,
                GURL("http://www.google.com")));
 }
@@ -642,3 +803,40 @@ TEST_F(SdchFilterTest, CanStillSetExactMatchDictionary) {
   EXPECT_TRUE(sdch_manager_->AddSdchDictionary(dictionary_text,
               GURL("http://" + dictionary_domain)));
 }
+
+// Make sure the DOS protection precludes the addition of too many dictionaries.
+TEST_F(SdchFilterTest, TooManyDictionaries) {
+  std::string dictionary_domain(".google.com");
+  std::string dictionary_text(NewSdchDictionary(dictionary_domain));
+
+  size_t count = 0;
+  while (count <= SdchManager::kMaxDictionaryCount + 1) {
+    if (!sdch_manager_->AddSdchDictionary(dictionary_text,
+                                          GURL("http://www.google.com")))
+      break;
+
+    dictionary_text += " ";  // Create dictionary with different SHA signature.
+    ++count;
+  }
+  EXPECT_EQ(SdchManager::kMaxDictionaryCount, count);
+}
+
+TEST_F(SdchFilterTest, DictionaryNotTooLarge) {
+  std::string dictionary_domain(".google.com");
+  std::string dictionary_text(NewSdchDictionary(dictionary_domain));
+
+  dictionary_text.append(
+      SdchManager::kMaxDictionarySize  - dictionary_text.size(), ' ');
+  EXPECT_TRUE(sdch_manager_->AddSdchDictionary(dictionary_text,
+              GURL("http://" + dictionary_domain)));
+}
+
+TEST_F(SdchFilterTest, DictionaryTooLarge) {
+  std::string dictionary_domain(".google.com");
+  std::string dictionary_text(NewSdchDictionary(dictionary_domain));
+
+  dictionary_text.append(
+      SdchManager::kMaxDictionarySize + 1 - dictionary_text.size(), ' ');
+  EXPECT_FALSE(sdch_manager_->AddSdchDictionary(dictionary_text,
+              GURL("http://" + dictionary_domain)));
+}
diff --git a/net/base/sdch_manager.cc b/net/base/sdch_manager.cc
index 005d300..75d0e29 100644
--- a/net/base/sdch_manager.cc
+++ b/net/base/sdch_manager.cc
@@ -17,6 +17,12 @@ using base::TimeDelta;
 
 //------------------------------------------------------------------------------
 // static
+const size_t SdchManager::kMaxDictionarySize = 100000;
+
+// static
+const size_t SdchManager::kMaxDictionaryCount = 20;
+
+// static
 SdchManager* SdchManager::global_;
 
 // static
@@ -85,8 +91,8 @@ const bool SdchManager::IsInSupportedDomain(const GURL& url) const {
   return blacklisted_domains_.end() == blacklisted_domains_.find(domain);
 }
 
-void SdchManager::FetchDictionary(const GURL& referring_url,
-                                  const GURL& dictionary_url) {
+bool SdchManager::CanFetchDictionary(const GURL& referring_url,
+                                     const GURL& dictionary_url) const {
   /* The user agent may retrieve a dictionary from the dictionary URL if all of 
      the following are true:
        1 The dictionary URL host name matches the referrer URL host name
@@ -100,12 +106,27 @@ void SdchManager::FetchDictionary(const GURL& referring_url,
   // I take "host name match" to be "is identical to"
   if (referring_url.host() != dictionary_url.host()) {
     SdchErrorRecovery(DICTIONARY_LOAD_ATTEMPT_FROM_DIFFERENT_HOST);
-    return;
+    return false;
   }
   if (referring_url.SchemeIs("https")) {
     SdchErrorRecovery(DICTIONARY_SELECTED_FOR_SSL);
-    return;
+    return false;
+  }
+
+  // TODO(jar): Remove this failsafe conservative hack which is more restrictive
+  // than current SDCH spec when needed, and justified by security audit.
+  if (!referring_url.SchemeIs("http")) {
+    SdchErrorRecovery(DICTIONARY_SELECTED_FROM_NON_HTTP);
+    return false;
   }
+
+  return true;
+}
+
+void SdchManager::FetchDictionary(const GURL& referring_url,
+                                  const GURL& dictionary_url) {
+  if (!CanFetchDictionary(referring_url, dictionary_url))
+    return;
   if (fetcher_.get())
     fetcher_->Schedule(dictionary_url);
 }
@@ -124,6 +145,11 @@ bool SdchManager::AddSdchDictionary(const std::string& dictionary_text,
   std::set<int> ports;
   Time expiration(Time::Now() + TimeDelta::FromDays(30));
 
+  if (dictionary_text.empty()) {
+    SdchErrorRecovery(DICTIONARY_HAS_NO_TEXT);
+    return false;  // Missing header.
+  }
+
   size_t header_end = dictionary_text.find("\n\n");
   if (std::string::npos == header_end) {
     SdchErrorRecovery(DICTIONARY_HAS_NO_HEADER);
@@ -176,6 +202,19 @@ bool SdchManager::AddSdchDictionary(const std::string& dictionary_text,
   if (!Dictionary::CanSet(domain, path, ports, dictionary_url))
     return false;
 
+  // TODO(jar): Remove these hacks to preclude a DOS attack involving piles of
+  // useless dictionaries.  We should probably have a cache eviction plan,
+  // instead of just blocking additions.  For now, with the spec in flux, it
+  // is probably not worth doing eviction handling.
+  if (kMaxDictionarySize < dictionary_text.size()) {
+    SdchErrorRecovery(DICTIONARY_IS_TOO_LARGE);
+    return false;
+  }
+  if (kMaxDictionaryCount <= dictionaries_.size()) {
+    SdchErrorRecovery(DICTIONARY_COUNT_EXCEEDED);
+    return false;
+  }
+
   UMA_HISTOGRAM_COUNTS(L"Sdch.Dictionary size loaded", dictionary_text.size());
   DLOG(INFO) << "Loaded dictionary with client hash " << client_hash <<
       " and server hash " << server_hash;
@@ -205,14 +244,19 @@ void SdchManager::GetVcdiffDictionary(const std::string& server_hash,
 // instances that can be used if/when a server specifies one.
 void SdchManager::GetAvailDictionaryList(const GURL& target_url,
                                          std::string* list) {
+  int count = 0;
   for (DictionaryMap::iterator it = dictionaries_.begin();
        it != dictionaries_.end(); ++it) {
     if (!it->second->CanAdvertise(target_url))
       continue;
+    ++count;
     if (!list->empty())
       list->append(",");
     list->append(it->second->client_hash());
   }
+  // Watch to see if we have corrupt or numerous dictionaries.
+  if (count > 0)
+    UMA_HISTOGRAM_COUNTS(L"Sdch.Advertisement_Count", count);
 }
 
 SdchManager::Dictionary::Dictionary(const std::string& dictionary_text,
@@ -348,6 +392,14 @@ bool SdchManager::Dictionary::CanUse(const GURL referring_url) {
     SdchErrorRecovery(DICTIONARY_FOUND_HAS_WRONG_SCHEME);
     return false;
   }
+
+  // TODO(jar): Remove overly restrictive failsafe test (added per security
+  // review) when we have a need to be more general.
+  if (!referring_url.SchemeIs("http")) {
+    SdchErrorRecovery(ATTEMPT_TO_DECODE_NON_HTTP_DATA);
+    return false;
+  }
+
   return true;
 }
 
diff --git a/net/base/sdch_manager.h b/net/base/sdch_manager.h
index 69055f1..3a8158406 100644
--- a/net/base/sdch_manager.h
+++ b/net/base/sdch_manager.h
@@ -85,15 +85,27 @@ class SdchManager {
     DICTIONARY_SPECIFIES_TOP_LEVEL_DOMAIN,
     DICTIONARY_DOMAIN_NOT_MATCHING_SOURCE_URL,
     DICTIONARY_PORT_NOT_MATCHING_SOURCE_URL,
+    DICTIONARY_HAS_NO_TEXT,
 
     // Dictionary loading problems.
     DICTIONARY_LOAD_ATTEMPT_FROM_DIFFERENT_HOST = 30,
     DICTIONARY_SELECTED_FOR_SSL,
     DICTIONARY_ALREADY_LOADED,
+    DICTIONARY_SELECTED_FROM_NON_HTTP,
+    DICTIONARY_IS_TOO_LARGE,
+    DICTIONARY_COUNT_EXCEEDED,
+
+    // Failsafe hack.
+    ATTEMPT_TO_DECODE_NON_HTTP_DATA = 40,
 
     MAX_PROBLEM_CODE  // Used to bound histogram.
   };
 
+  // Use the following static limits to block DOS attacks until we implement
+  // a cached dictionary evicition strategy.
+  static const size_t kMaxDictionarySize;
+  static const size_t kMaxDictionaryCount;
+
   // There is one instance of |Dictionary| for each memory-cached SDCH
   // dictionary.
   class Dictionary : public base::RefCounted<Dictionary> {
@@ -197,6 +209,11 @@ class SdchManager {
   // cached in memory.
   void FetchDictionary(const GURL& referring_url, const GURL& dictionary_url);
 
+  // Security test function used before initiating a fetch.
+  // Return true if fetch is legal.
+  bool CanFetchDictionary(const GURL& referring_url,
+                          const GURL& dictionary_url) const;
+
   // Add an SDCH dictionary to our list of availible dictionaries. This addition
   // will fail (return false) if addition is illegal (data in the dictionary is
   // not acceptable from the dictionary_url; dictionary already added, etc.).