Skip to content

Commit

Permalink
When parsing cookie expiration times, saturate out of range dates
Browse files Browse the repository at this point in the history 
rather than reject them.

Roughly this means cookie expiration times prior to 1970 on (non-OSX)
POSIX, or after 2038 on 32-bit (non-OSX) POSIX, will now be interpreted
as either very small or very large base::Time values.

BUG=649416

Review-Url: https://chromiumcodereview.appspot.com/2424443002
Cr-Commit-Position: refs/heads/master@{#426213}
  • Loading branch information
mmenke authored and Commit bot committed Oct 19, 2016
1 parent 6d4219f commit 93cde27
Show file tree
Hide file tree
Showing 7 changed files with 225 additions and 85 deletions.
6 changes: 4 additions & 2 deletions extensions/browser/api/web_request/web_request_api_helpers.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,10 @@ bool ParseCookieLifetime(net::ParsedCookie* cookie,
}

Time parsed_expiry_time;
if (cookie->HasExpires())
parsed_expiry_time = net::cookie_util::ParseCookieTime(cookie->Expires());
if (cookie->HasExpires()) {
parsed_expiry_time =
net::cookie_util::ParseCookieExpirationTime(cookie->Expires());
}

if (!parsed_expiry_time.is_null()) {
*seconds_till_expiry =
Expand Down
3 changes: 2 additions & 1 deletion net/cookies/canonical_cookie.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,8 @@ Time CanonicalCookie::CanonExpiration(const ParsedCookie& pc,
// Try the Expires attribute.
if (pc.HasExpires() && !pc.Expires().empty()) {
// Adjust for clock skew between server and host.
base::Time parsed_expiry = cookie_util::ParseCookieTime(pc.Expires());
base::Time parsed_expiry =
cookie_util::ParseCookieExpirationTime(pc.Expires());
if (!parsed_expiry.is_null())
return parsed_expiry + (current - server_time);
}
Expand Down
2 changes: 2 additions & 0 deletions net/cookies/canonical_cookie.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ class NET_EXPORT CanonicalCookie {
std::string DebugString() const;

static std::string CanonPath(const GURL& url, const ParsedCookie& pc);

// Returns a "null" time if expiration was unspecified or invalid.
static base::Time CanonExpiration(const ParsedCookie& pc,
const base::Time& current,
const base::Time& server_time);
Expand Down
6 changes: 3 additions & 3 deletions net/cookies/cookie_monster_store_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,9 @@ std::unique_ptr<CanonicalCookie> BuildCanonicalCookie(
// functions. Would be nice to export them, and re-use here.
EXPECT_FALSE(pc.HasMaxAge());
EXPECT_TRUE(pc.HasPath());
base::Time cookie_expires = pc.HasExpires()
? cookie_util::ParseCookieTime(pc.Expires())
: base::Time();
base::Time cookie_expires =
pc.HasExpires() ? cookie_util::ParseCookieExpirationTime(pc.Expires())
: base::Time();
std::string cookie_path = pc.Path();

return CanonicalCookie::Create(url, pc.Name(), pc.Value(), url.host(),
Expand Down
88 changes: 80 additions & 8 deletions net/cookies/cookie_util.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,80 @@
namespace net {
namespace cookie_util {

namespace {

base::Time MinNonNullTime() {
return base::Time::FromInternalValue(1);
}

// Tries to assemble a base::Time given a base::Time::Exploded representing a
// UTC calendar date.
//
// If the date falls outside of the range supported internally by
// FromUTCExploded(), then the result is clamped to the range that
// FromUTCExploded() supports on the current platform.
bool SaturatedTimeFromUTCExploded(const base::Time::Exploded& exploded,
base::Time* out) {
// Try to calculate the base::Time in the normal fashion.
if (base::Time::FromUTCExploded(exploded, out)) {
// Don't return Time(0) on success.
if (out->is_null())
*out = MinNonNullTime();
return true;
}

// base::Time::FromUTCExploded() has platform-specific limits:
//
// * Windows: Years 1601 - 30827
// * 32-bit POSIX: Years 1970 - 2038
//
// Work around this by clamping values when imploding the time is doomed
// to fail.
//
// Note that the following implementation is NOT perfect. It will accept
// some invalid calendar dates in the out-of-range case.
if (!exploded.HasValidValues())
return false;
#if defined(OS_POSIX) && !defined(OS_MACOSX)
// Allow dates prior to unix epoch (which fail on non-Mac/iOS POSIX).
if (exploded.year < 1970) {
*out = base::Time::UnixEpoch();
return true;
}

// On 32-bit non-Mac/iOS POSIX systems, the time_t value that FromExploded()
// returns overflows in the middle of year 2038. In that case, return the max
// value that can be represented by a 32-bit time_t.
if (sizeof(time_t) == 4u && exploded.year >= 2038) {
*out = base::Time::FromTimeT(std::numeric_limits<time_t>::max());
return true;
}
#endif // defined(OS_POSIX) && !defined(OS_MACOSX)

#if defined(OS_WIN)
// Allow dates prior to Windows epoch.
if (exploded.year < 1601) {
*out = MinNonNullTime();
return true;
}

// Allow dates after the Windows epoch.
if (exploded.year >= 30827) {
// This is the maximum value a FILETIME can represent, though FromExploded()
// does fail on marginally smaller FILETIME values. The division by 10 is
// needed because FILETIMEs are in terms of hundreds of nanoseconds.
// This relies on base::Time() returning the start of the Windows epoch.
*out =
base::Time::FromInternalValue(std::numeric_limits<int64_t>::max() / 10);
return true;
}
#endif // defined(OS_WIN)

return false;
}

} // namespace

bool DomainIsHostOnly(const std::string& domain_string) {
return (domain_string.empty() || domain_string[0] != '.');
}
Expand Down Expand Up @@ -103,7 +177,7 @@ bool GetCookieDomainWithString(const GURL& url,
// - The time must be of the format hh:mm:ss.
// An average cookie expiration will look something like this:
// Sat, 15-Apr-17 21:01:22 GMT
base::Time ParseCookieTime(const std::string& time_string) {
base::Time ParseCookieExpirationTime(const std::string& time_string) {
static const char* const kMonths[] = {
"jan", "feb", "mar", "apr", "may", "jun",
"jul", "aug", "sep", "oct", "nov", "dec" };
Expand Down Expand Up @@ -200,13 +274,11 @@ base::Time ParseCookieTime(const std::string& time_string) {
if (exploded.year >= 0 && exploded.year <= 68)
exploded.year += 2000;

// If our values are within their correct ranges, we got our time.
if (exploded.day_of_month >= 1 && exploded.day_of_month <= 31 &&
exploded.month >= 1 && exploded.month <= 12 &&
exploded.year >= 1601 && exploded.year <= 30827 &&
exploded.hour <= 23 && exploded.minute <= 59 && exploded.second <= 59) {
return base::Time::FromUTCExploded(exploded);
}
// Note that clipping the date if it is outside of a platform-specific range
// is permitted by: https://tools.ietf.org/html/rfc6265#section-5.2.1
base::Time result;
if (SaturatedTimeFromUTCExploded(exploded, &result))
return result;

// One of our values was out of expected range. For well-formed input,
// the following check would be reasonable:
Expand Down
15 changes: 13 additions & 2 deletions net/cookies/cookie_util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,19 @@ NET_EXPORT bool GetCookieDomainWithString(const GURL& url,
// i.e. it doesn't begin with a leading '.' character.
NET_EXPORT bool DomainIsHostOnly(const std::string& domain_string);

// Parses the string with the cookie time (very forgivingly).
NET_EXPORT base::Time ParseCookieTime(const std::string& time_string);
// Parses the string with the cookie expiration time (very forgivingly).
// Returns the "null" time on failure.
//
// If the expiration date is either too small or too large (there are
// platform-specific limits for allowed date ranges), then it will be clipped
// to the min/max time Time::FromUTCExploded() can return on the current
// platform,
// rather than failing. In particular:
//
// On non-Mac 32-bit POSIX, times will currently be clamped between 1970 and
// 2038.
// On Windows, times will currently be clamped between year 1601 and 30827.
NET_EXPORT base::Time ParseCookieExpirationTime(const std::string& time_string);

// Convenience for converting a cookie origin (domain and https pair) to a URL.
NET_EXPORT GURL CookieOriginToURL(const std::string& domain, bool is_https);
Expand Down
190 changes: 121 additions & 69 deletions net/cookies/cookie_util_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,79 +63,81 @@ TEST(CookieUtilTest, TestCookieDateParsing) {
const bool valid;
const time_t epoch;
} tests[] = {
{ "Sat, 15-Apr-17 21:01:22 GMT", true, 1492290082 },
{ "Thu, 19-Apr-2007 16:00:00 GMT", true, 1176998400 },
{ "Wed, 25 Apr 2007 21:02:13 GMT", true, 1177534933 },
{ "Thu, 19/Apr\\2007 16:00:00 GMT", true, 1176998400 },
{ "Fri, 1 Jan 2010 01:01:50 GMT", true, 1262307710 },
{ "Wednesday, 1-Jan-2003 00:00:00 GMT", true, 1041379200 },
{ ", 1-Jan-2003 00:00:00 GMT", true, 1041379200 },
{ " 1-Jan-2003 00:00:00 GMT", true, 1041379200 },
{ "1-Jan-2003 00:00:00 GMT", true, 1041379200 },
{ "Wed,18-Apr-07 22:50:12 GMT", true, 1176936612 },
{ "WillyWonka , 18-Apr-07 22:50:12 GMT", true, 1176936612 },
{ "WillyWonka , 18-Apr-07 22:50:12", true, 1176936612 },
{ "WillyWonka , 18-apr-07 22:50:12", true, 1176936612 },
{ "Mon, 18-Apr-1977 22:50:13 GMT", true, 230251813 },
{ "Mon, 18-Apr-77 22:50:13 GMT", true, 230251813 },
// If the cookie came in with the expiration quoted (which in terms of
// the RFC you shouldn't do), we will get string quoted. Bug 1261605.
{ "\"Sat, 15-Apr-17\\\"21:01:22\\\"GMT\"", true, 1492290082 },
// Test with full month names and partial names.
{ "Partyday, 18- April-07 22:50:12", true, 1176936612 },
{ "Partyday, 18 - Apri-07 22:50:12", true, 1176936612 },
{ "Wednes, 1-Januar-2003 00:00:00 GMT", true, 1041379200 },
// Test that we always take GMT even with other time zones or bogus
// values. The RFC says everything should be GMT, and in the worst case
// we are 24 hours off because of zone issues.
{ "Sat, 15-Apr-17 21:01:22", true, 1492290082 },
{ "Sat, 15-Apr-17 21:01:22 GMT-2", true, 1492290082 },
{ "Sat, 15-Apr-17 21:01:22 GMT BLAH", true, 1492290082 },
{ "Sat, 15-Apr-17 21:01:22 GMT-0400", true, 1492290082 },
{ "Sat, 15-Apr-17 21:01:22 GMT-0400 (EDT)",true, 1492290082 },
{ "Sat, 15-Apr-17 21:01:22 DST", true, 1492290082 },
{ "Sat, 15-Apr-17 21:01:22 -0400", true, 1492290082 },
{ "Sat, 15-Apr-17 21:01:22 (hello there)", true, 1492290082 },
// Test that if we encounter multiple : fields, that we take the first
// that correctly parses.
{ "Sat, 15-Apr-17 21:01:22 11:22:33", true, 1492290082 },
{ "Sat, 15-Apr-17 ::00 21:01:22", true, 1492290082 },
{ "Sat, 15-Apr-17 boink:z 21:01:22", true, 1492290082 },
// We take the first, which in this case is invalid.
{ "Sat, 15-Apr-17 91:22:33 21:01:22", false, 0 },
// amazon.com formats their cookie expiration like this.
{ "Thu Apr 18 22:50:12 2007 GMT", true, 1176936612 },
// Test that hh:mm:ss can occur anywhere.
{ "22:50:12 Thu Apr 18 2007 GMT", true, 1176936612 },
{ "Thu 22:50:12 Apr 18 2007 GMT", true, 1176936612 },
{ "Thu Apr 22:50:12 18 2007 GMT", true, 1176936612 },
{ "Thu Apr 18 22:50:12 2007 GMT", true, 1176936612 },
{ "Thu Apr 18 2007 22:50:12 GMT", true, 1176936612 },
{ "Thu Apr 18 2007 GMT 22:50:12", true, 1176936612 },
// Test that the day and year can be anywhere if they are unambigious.
{ "Sat, 15-Apr-17 21:01:22 GMT", true, 1492290082 },
{ "15-Sat, Apr-17 21:01:22 GMT", true, 1492290082 },
{ "15-Sat, Apr 21:01:22 GMT 17", true, 1492290082 },
{ "15-Sat, Apr 21:01:22 GMT 2017", true, 1492290082 },
{ "15 Apr 21:01:22 2017", true, 1492290082 },
{ "15 17 Apr 21:01:22", true, 1492290082 },
{ "Apr 15 17 21:01:22", true, 1492290082 },
{ "Apr 15 21:01:22 17", true, 1492290082 },
{ "2017 April 15 21:01:22", true, 1492290082 },
{ "15 April 2017 21:01:22", true, 1492290082 },
// Some invalid dates
{ "98 April 17 21:01:22", false, 0 },
{ "Thu, 012-Aug-2008 20:49:07 GMT", false, 0 },
{ "Thu, 12-Aug-31841 20:49:07 GMT", false, 0 },
{ "Thu, 12-Aug-9999999999 20:49:07 GMT", false, 0 },
{ "Thu, 999999999999-Aug-2007 20:49:07 GMT", false, 0 },
{ "Thu, 12-Aug-2007 20:61:99999999999 GMT", false, 0 },
{ "IAintNoDateFool", false, 0 },
{"Sat, 15-Apr-17 21:01:22 GMT", true, 1492290082},
{"Thu, 19-Apr-2007 16:00:00 GMT", true, 1176998400},
{"Wed, 25 Apr 2007 21:02:13 GMT", true, 1177534933},
{"Thu, 19/Apr\\2007 16:00:00 GMT", true, 1176998400},
{"Fri, 1 Jan 2010 01:01:50 GMT", true, 1262307710},
{"Wednesday, 1-Jan-2003 00:00:00 GMT", true, 1041379200},
{", 1-Jan-2003 00:00:00 GMT", true, 1041379200},
{" 1-Jan-2003 00:00:00 GMT", true, 1041379200},
{"1-Jan-2003 00:00:00 GMT", true, 1041379200},
{"Wed,18-Apr-07 22:50:12 GMT", true, 1176936612},
{"WillyWonka , 18-Apr-07 22:50:12 GMT", true, 1176936612},
{"WillyWonka , 18-Apr-07 22:50:12", true, 1176936612},
{"WillyWonka , 18-apr-07 22:50:12", true, 1176936612},
{"Mon, 18-Apr-1977 22:50:13 GMT", true, 230251813},
{"Mon, 18-Apr-77 22:50:13 GMT", true, 230251813},
// If the cookie came in with the expiration quoted (which in terms of
// the RFC you shouldn't do), we will get string quoted. Bug 1261605.
{"\"Sat, 15-Apr-17\\\"21:01:22\\\"GMT\"", true, 1492290082},
// Test with full month names and partial names.
{"Partyday, 18- April-07 22:50:12", true, 1176936612},
{"Partyday, 18 - Apri-07 22:50:12", true, 1176936612},
{"Wednes, 1-Januar-2003 00:00:00 GMT", true, 1041379200},
// Test that we always take GMT even with other time zones or bogus
// values. The RFC says everything should be GMT, and in the worst case
// we are 24 hours off because of zone issues.
{"Sat, 15-Apr-17 21:01:22", true, 1492290082},
{"Sat, 15-Apr-17 21:01:22 GMT-2", true, 1492290082},
{"Sat, 15-Apr-17 21:01:22 GMT BLAH", true, 1492290082},
{"Sat, 15-Apr-17 21:01:22 GMT-0400", true, 1492290082},
{"Sat, 15-Apr-17 21:01:22 GMT-0400 (EDT)", true, 1492290082},
{"Sat, 15-Apr-17 21:01:22 DST", true, 1492290082},
{"Sat, 15-Apr-17 21:01:22 -0400", true, 1492290082},
{"Sat, 15-Apr-17 21:01:22 (hello there)", true, 1492290082},
// Test that if we encounter multiple : fields, that we take the first
// that correctly parses.
{"Sat, 15-Apr-17 21:01:22 11:22:33", true, 1492290082},
{"Sat, 15-Apr-17 ::00 21:01:22", true, 1492290082},
{"Sat, 15-Apr-17 boink:z 21:01:22", true, 1492290082},
// We take the first, which in this case is invalid.
{"Sat, 15-Apr-17 91:22:33 21:01:22", false, 0},
// amazon.com formats their cookie expiration like this.
{"Thu Apr 18 22:50:12 2007 GMT", true, 1176936612},
// Test that hh:mm:ss can occur anywhere.
{"22:50:12 Thu Apr 18 2007 GMT", true, 1176936612},
{"Thu 22:50:12 Apr 18 2007 GMT", true, 1176936612},
{"Thu Apr 22:50:12 18 2007 GMT", true, 1176936612},
{"Thu Apr 18 22:50:12 2007 GMT", true, 1176936612},
{"Thu Apr 18 2007 22:50:12 GMT", true, 1176936612},
{"Thu Apr 18 2007 GMT 22:50:12", true, 1176936612},
// Test that the day and year can be anywhere if they are unambigious.
{"Sat, 15-Apr-17 21:01:22 GMT", true, 1492290082},
{"15-Sat, Apr-17 21:01:22 GMT", true, 1492290082},
{"15-Sat, Apr 21:01:22 GMT 17", true, 1492290082},
{"15-Sat, Apr 21:01:22 GMT 2017", true, 1492290082},
{"15 Apr 21:01:22 2017", true, 1492290082},
{"15 17 Apr 21:01:22", true, 1492290082},
{"Apr 15 17 21:01:22", true, 1492290082},
{"Apr 15 21:01:22 17", true, 1492290082},
{"2017 April 15 21:01:22", true, 1492290082},
{"15 April 2017 21:01:22", true, 1492290082},
// Some invalid dates
{"98 April 17 21:01:22", false, 0},
{"Thu, 012-Aug-2008 20:49:07 GMT", false, 0},
{"Thu, 12-Aug-9999999999 20:49:07 GMT", false, 0},
{"Thu, 999999999999-Aug-2007 20:49:07 GMT", false, 0},
{"Thu, 12-Aug-2007 20:61:99999999999 GMT", false, 0},
{"IAintNoDateFool", false, 0},
{"1600 April 33 21:01:22", false, 0},
{"1970 April 33 21:01:22", false, 0},
{"Thu, 33-Aug-31841 20:49:07 GMT", false, 0},
};

base::Time parsed_time;
for (size_t i = 0; i < arraysize(tests); ++i) {
parsed_time = cookie_util::ParseCookieTime(tests[i].str);
parsed_time = cookie_util::ParseCookieExpirationTime(tests[i].str);
if (!tests[i].valid) {
EXPECT_TRUE(parsed_time.is_null()) << tests[i].str;
continue;
Expand All @@ -145,6 +147,56 @@ TEST(CookieUtilTest, TestCookieDateParsing) {
}
}

// Tests parsing dates that are beyond 2038. 32-bit (non-Mac) POSIX systems are
// incapable of doing this, however the expectation is for cookie parsing to
// succeed anyway (and return the minimum value Time::FromUTCExploded() can
// parse on the current platform).
TEST(CookieUtilTest, ParseCookieExpirationTimeBeyond2038) {
const char* kTests[] = {
"Thu, 12-Aug-31841 20:49:07 GMT", "2039 April 15 21:01:22",
"2039 April 15 21:01:22", "2038 April 15 21:01:22",
};

for (const auto& test : kTests) {
base::Time parsed_time = cookie_util::ParseCookieExpirationTime(test);
EXPECT_FALSE(parsed_time.is_null());

// It should either have an exact value, or the maximum value that
// Time::FromeUTCExploded() can return on the current platform. For
// simplicity just check that it is greater than an arbitray date.
base::Time almost_jan_2038 =
base::Time::UnixEpoch() + base::TimeDelta::FromDays(365 * 68);
EXPECT_LT(almost_jan_2038, parsed_time);

base::Time almost_jan_32000 =
base::Time::UnixEpoch() + base::TimeDelta::FromDays(365 * 32030);
EXPECT_LT(parsed_time, almost_jan_32000);
}
}

// Tests parsing dates that are prior to (or around) 1970. POSIX systems are
// incapable of doing this, however the expectation is for cookie parsing to
// succeed anyway (and return a minimal base::Time).
TEST(CookieUtilTest, ParseCookieExpirationTimeBefore1970) {
const char* kTests[] = {
// The unix epoch.
"1970 Jan 1 00:00:00",
// The windows epoch.
"1601 Jan 1 00:00:00",
// Other dates.
"1969 March 3 21:01:22", "1600 April 15 21:01:22",
};

for (const auto& test : kTests) {
base::Time parsed_time = cookie_util::ParseCookieExpirationTime(test);
EXPECT_FALSE(parsed_time.is_null());

// It should either have an exact value, or should be base::Time(1)
// For simplicity just check that it is less than the unix epoch.
EXPECT_LE(parsed_time, base::Time::UnixEpoch());
}
}

TEST(CookieUtilTest, TestRequestCookieParsing) {
std::vector<RequestCookieParsingTest> tests;

Expand Down

0 comments on commit 93cde27

Please sign in to comment.