Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(505)

Unified Diff: base/time/time_posix.cc

Issue 2405453002: Fix Integer-overflow in base::Time::FromExploded. (Closed)
Patch Set: rebased and fixed net unittest Created 4 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « base/time/time_mac.cc ('k') | base/time/time_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: base/time/time_posix.cc
diff --git a/base/time/time_posix.cc b/base/time/time_posix.cc
index ac0e99f7eb61b30de3eb16556f77869a8ee4704b..4caf3866c524fd886f24c750a13fd5a8157f2207 100644
--- a/base/time/time_posix.cc
+++ b/base/time/time_posix.cc
@@ -242,7 +242,6 @@ bool Time::FromExploded(bool is_local, const Exploded& exploded, Time* time) {
timestruct.tm_zone = NULL; // not a POSIX field, so mktime/timegm ignore
#endif
- int64_t milliseconds;
SysTime seconds;
// Certain exploded dates do not really exist due to daylight saving times,
@@ -280,6 +279,7 @@ bool Time::FromExploded(bool is_local, const Exploded& exploded, Time* time) {
// return is the best that can be done here. It's not ideal, but it's better
// than failing here or ignoring the overflow case and treating each time
// overflow as one second prior to the epoch.
+ int64_t milliseconds = 0;
if (seconds == -1 &&
(exploded.year < 1969 || exploded.year > 1970)) {
// If exploded.year is 1969 or 1970, take -1 as correct, with the
@@ -312,13 +312,25 @@ bool Time::FromExploded(bool is_local, const Exploded& exploded, Time* time) {
milliseconds += (kMillisecondsPerSecond - 1);
}
} else {
- milliseconds = seconds * kMillisecondsPerSecond + exploded.millisecond;
+ base::CheckedNumeric<int64_t> checked_millis = seconds;
+ checked_millis *= kMillisecondsPerSecond;
+ checked_millis += exploded.millisecond;
+ if (!checked_millis.IsValid()) {
+ *time = base::Time(0);
+ return false;
+ }
+ milliseconds = checked_millis.ValueOrDie();
}
- // Adjust from Unix (1970) to Windows (1601) epoch.
- base::Time converted_time =
- Time((milliseconds * kMicrosecondsPerMillisecond) +
- kWindowsEpochDeltaMicroseconds);
+ // Adjust from Unix (1970) to Windows (1601) epoch avoiding overflows.
+ base::CheckedNumeric<int64_t> checked_microseconds_win_epoch = milliseconds;
+ checked_microseconds_win_epoch *= kMicrosecondsPerMillisecond;
+ checked_microseconds_win_epoch += kWindowsEpochDeltaMicroseconds;
+ if (!checked_microseconds_win_epoch.IsValid()) {
+ *time = base::Time(0);
+ return false;
+ }
+ base::Time converted_time(checked_microseconds_win_epoch.ValueOrDie());
// If |exploded.day_of_month| is set to 31 on a 28-30 day month, it will
// return the first day of the next month. Thus round-trip the time and
« no previous file with comments | « base/time/time_mac.cc ('k') | base/time/time_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698