From 337a405cd61e11152f8dd5f5503ef6fc98e24905 Mon Sep 17 00:00:00 2001
From: "agl@chromium.org"
 <agl@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Tue, 30 Nov 2010 15:09:33 +0000
Subject: net: limit HSTS ages to one year.

BUG=64635
TEST=net_unittests

http://codereview.chromium.org/5376005/

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@67709 0039d316-1c4b-4281-b951-d872f2087c98
---
 net/base/transport_security_state.cc          | 30 +++++++++++++++++++++------
 net/base/transport_security_state.h           |  3 +++
 net/base/transport_security_state_unittest.cc | 13 ++++++++++--
 3 files changed, 38 insertions(+), 8 deletions(-)

(limited to 'net/base')

diff --git a/net/base/transport_security_state.cc b/net/base/transport_security_state.cc
index 3014e21..5cf85a1 100644
--- a/net/base/transport_security_state.cc
+++ b/net/base/transport_security_state.cc
@@ -20,6 +20,8 @@
 
 namespace net {
 
+const long int TransportSecurityState::kMaxHSTSAgeSecs = 86400 * 365;  // 1 year
+
 TransportSecurityState::TransportSecurityState()
     : delegate_(NULL) {
 }
@@ -98,6 +100,24 @@ bool TransportSecurityState::IsEnabledForHost(DomainState* result,
   return false;
 }
 
+// MaxAgeToInt converts a string representation of a number of seconds into a
+// int. We use strtol in order to handle overflow correctly. The string may
+// contain an arbitary number which we should truncate correctly rather than
+// throwing a parse failure.
+static bool MaxAgeToInt(std::string::const_iterator begin,
+                        std::string::const_iterator end,
+                        int* result) {
+  const std::string s(begin, end);
+  char* endptr;
+  long int i = strtol(s.data(), &endptr, 10 /* base */);
+  if (*endptr || i < 0)
+    return false;
+  if (i > TransportSecurityState::kMaxHSTSAgeSecs)
+    i = TransportSecurityState::kMaxHSTSAgeSecs;
+  *result = i;
+  return true;
+}
+
 // "Strict-Transport-Security" ":"
 //     "max-age" "=" delta-seconds [ ";" "includeSubDomains" ]
 bool TransportSecurityState::ParseHeader(const std::string& value,
@@ -106,7 +126,7 @@ bool TransportSecurityState::ParseHeader(const std::string& value,
   DCHECK(max_age);
   DCHECK(include_subdomains);
 
-  int max_age_candidate;
+  int max_age_candidate = 0;
 
   enum ParserState {
     START,
@@ -142,11 +162,9 @@ bool TransportSecurityState::ParseHeader(const std::string& value,
       case AFTER_MAX_AGE_EQUALS:
         if (IsAsciiWhitespace(*tokenizer.token_begin()))
           continue;
-        if (!base::StringToInt(tokenizer.token_begin(),
-                               tokenizer.token_end(),
-                               &max_age_candidate))
-          return false;
-        if (max_age_candidate < 0)
+        if (!MaxAgeToInt(tokenizer.token_begin(),
+                         tokenizer.token_end(),
+                         &max_age_candidate))
           return false;
         state = AFTER_MAX_AGE;
         break;
diff --git a/net/base/transport_security_state.h b/net/base/transport_security_state.h
index 49b44d7..fcd4e79 100644
--- a/net/base/transport_security_state.h
+++ b/net/base/transport_security_state.h
@@ -88,6 +88,9 @@ class TransportSecurityState :
   bool Serialise(std::string* output);
   bool Deserialise(const std::string& state, bool* dirty);
 
+  // The maximum number of seconds for which we'll cache an HSTS request.
+  static const long int kMaxHSTSAgeSecs;
+
  private:
   friend class base::RefCountedThreadSafe<TransportSecurityState>;
   FRIEND_TEST_ALL_PREFIXES(TransportSecurityStateTest, IsPreloaded);
diff --git a/net/base/transport_security_state_unittest.cc b/net/base/transport_security_state_unittest.cc
index 2a06501..00ecef5 100644
--- a/net/base/transport_security_state_unittest.cc
+++ b/net/base/transport_security_state_unittest.cc
@@ -116,18 +116,27 @@ TEST_F(TransportSecurityStateTest, ValidHeaders) {
 
   EXPECT_TRUE(TransportSecurityState::ParseHeader(
       "max-age=39408299  ;incLudesUbdOmains", &max_age, &include_subdomains));
-  EXPECT_EQ(max_age, 39408299);
+  EXPECT_EQ(max_age,
+            std::min(TransportSecurityState::kMaxHSTSAgeSecs, 39408299l));
   EXPECT_TRUE(include_subdomains);
 
   EXPECT_TRUE(TransportSecurityState::ParseHeader(
       "max-age=394082038  ;  incLudesUbdOmains", &max_age, &include_subdomains));
-  EXPECT_EQ(max_age, 394082038);
+  EXPECT_EQ(max_age,
+            std::min(TransportSecurityState::kMaxHSTSAgeSecs, 394082038l));
   EXPECT_TRUE(include_subdomains);
 
   EXPECT_TRUE(TransportSecurityState::ParseHeader(
       "  max-age=0  ;  incLudesUbdOmains   ", &max_age, &include_subdomains));
   EXPECT_EQ(max_age, 0);
   EXPECT_TRUE(include_subdomains);
+
+  EXPECT_TRUE(TransportSecurityState::ParseHeader(
+      "  max-age=999999999999999999999999999999999999999999999  ;"
+      "  incLudesUbdOmains   ",
+      &max_age, &include_subdomains));
+  EXPECT_EQ(max_age, TransportSecurityState::kMaxHSTSAgeSecs);
+  EXPECT_TRUE(include_subdomains);
 }
 
 TEST_F(TransportSecurityStateTest, SimpleMatches) {
-- 
cgit v1.1