0

Fix Integer-overflow in base::Time::FromExploded.

The old implementation doesn't handle possible overflows,
when year is too large, for example. It makes a result
to be larger than 2^63 - 1, which results in overflow.

Fix Posix: use safe_math.h for multiplication and addition. If
overflow occurs, return possibly maximum platform dependent
value.

Fix Mac and Win: if safe cast is impossible, return Time(0).

Fix media and components: use day of week as well, as long
as unused variable results in undefined behavior and overflow

BUG=653445

Review-Url: https://codereview.chromium.org/2405453002
Cr-Commit-Position: refs/heads/master@{#427064}
This commit is contained in:
maksim.sisov
2016-10-24 07:07:27 -07:00
committed by Commit bot
parent 5165ccf083
commit eebbdecabe
10 changed files with 63 additions and 21 deletions

@ -545,7 +545,7 @@ class BASE_EXPORT Time : public time_internal::TimeBase<Time> {
// Converts an exploded structure representing either the local time or UTC
// into a Time class. Returns false on a failure when, for example, a day of
// month is set to 31 on a 28-30 day month.
// month is set to 31 on a 28-30 day month. Returns Time(0) on overflow.
static bool FromUTCExploded(const Exploded& exploded,
Time* time) WARN_UNUSED_RESULT {
return FromExploded(false, exploded, time);

@ -190,9 +190,18 @@ bool Time::FromExploded(bool is_local, const Exploded& exploded, Time* time) {
exploded.millisecond);
CFAbsoluteTime seconds = absolute_time + kCFAbsoluteTimeIntervalSince1970;
base::Time converted_time =
Time(static_cast<int64_t>(seconds * kMicrosecondsPerSecond) +
kWindowsEpochDeltaMicroseconds);
// CFAbsolutTime is typedef of double. Convert seconds to
// microseconds and then cast to int64. If
// it cannot be suited to int64, then fail to avoid overflows.
double microseconds =
(seconds * kMicrosecondsPerSecond) + kWindowsEpochDeltaMicroseconds;
if (microseconds > std::numeric_limits<int64_t>::max() ||
microseconds < std::numeric_limits<int64_t>::min()) {
*time = Time(0);
return false;
}
base::Time converted_time = Time(static_cast<int64_t>(microseconds));
// 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.

@ -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

@ -54,6 +54,11 @@ TEST(TimeTestOutOfBounds, FromExplodedOutOfBoundsTime) {
{{2016, 10, 0, 25, 7, 47, 234, 0}, false},
// Milliseconds are too large
{{2016, 10, 0, 25, 6, 31, 23, 1643}, false},
// Test overflow. Time is valid, but overflow case
// results in Time(0).
{{9840633, 1, 0, 1, 1, 1, 0, 0}, true},
// Underflow will fail as well.
{{-9840633, 1, 0, 1, 1, 1, 0, 0}, true},
};
for (const auto& test : kDateTestData) {

@ -110,6 +110,12 @@ uint64_t QPCNowRaw() {
return perf_counter_now.QuadPart;
}
bool SafeConvertToWord(int in, WORD* out) {
base::CheckedNumeric<WORD> result = in;
*out = result.ValueOrDefault(std::numeric_limits<WORD>::max());
return result.IsValid();
}
} // namespace
// Time -----------------------------------------------------------------------
@ -238,16 +244,20 @@ bool Time::IsHighResolutionTimerInUse() {
// static
bool Time::FromExploded(bool is_local, const Exploded& exploded, Time* time) {
// Create the system struct representing our exploded time. It will either be
// in local time or UTC.
// in local time or UTC.If casting from int to WORD results in overflow,
// fail and return Time(0).
SYSTEMTIME st;
st.wYear = static_cast<WORD>(exploded.year);
st.wMonth = static_cast<WORD>(exploded.month);
st.wDayOfWeek = static_cast<WORD>(exploded.day_of_week);
st.wDay = static_cast<WORD>(exploded.day_of_month);
st.wHour = static_cast<WORD>(exploded.hour);
st.wMinute = static_cast<WORD>(exploded.minute);
st.wSecond = static_cast<WORD>(exploded.second);
st.wMilliseconds = static_cast<WORD>(exploded.millisecond);
if (!SafeConvertToWord(exploded.year, &st.wYear) ||
!SafeConvertToWord(exploded.month, &st.wMonth) ||
!SafeConvertToWord(exploded.day_of_week, &st.wDayOfWeek) ||
!SafeConvertToWord(exploded.day_of_month, &st.wDay) ||
!SafeConvertToWord(exploded.hour, &st.wHour) ||
!SafeConvertToWord(exploded.minute, &st.wMinute) ||
!SafeConvertToWord(exploded.second, &st.wSecond) ||
!SafeConvertToWord(exploded.millisecond, &st.wMilliseconds)) {
*time = base::Time(0);
return false;
}
FILETIME ft;
bool success = true;

@ -29,6 +29,7 @@ base::Time::Exploded TestExplodedTime() {
exploded.year = 2001;
exploded.month = 12;
exploded.day_of_month = 31;
exploded.day_of_week = 1;
exploded.hour = 12;
exploded.minute = 1;
exploded.second = 0;

@ -753,6 +753,9 @@ bool FFmpegUTCDateToTime(const char* date_utc, base::Time* out) {
base::Time::Exploded exploded;
exploded.millisecond = 0;
// This field cannot be uninitialized. Unless not modified, make it 0 here
// then.
exploded.day_of_week = 0;
if (base::StringToInt(date_fields[0], &exploded.year) &&
base::StringToInt(date_fields[1], &exploded.month) &&
base::StringToInt(date_fields[2], &exploded.day_of_month) &&

@ -86,6 +86,7 @@ bool WebMInfoParser::OnBinary(int id, const uint8_t* data, int size) {
exploded_epoch.year = 2001;
exploded_epoch.month = 1;
exploded_epoch.day_of_month = 1;
exploded_epoch.day_of_week = 1;
exploded_epoch.hour = 0;
exploded_epoch.minute = 0;
exploded_epoch.second = 0;

@ -173,6 +173,7 @@ static base::Time kLiveTimelineOffset() {
exploded_time.year = 2012;
exploded_time.month = 11;
exploded_time.day_of_month = 10;
exploded_time.day_of_week = 6;
exploded_time.hour = 12;
exploded_time.minute = 34;
exploded_time.second = 56;

@ -964,10 +964,10 @@ TEST(HttpUtilTest, ParseRanges) {
}
TEST(HttpUtilTest, ParseRetryAfterHeader) {
base::Time::Exploded now_exploded = { 2014, 11, -1, 5, 22, 39, 30, 0 };
base::Time::Exploded now_exploded = {2014, 11, 4, 5, 22, 39, 30, 0};
base::Time now = base::Time::FromUTCExploded(now_exploded);
base::Time::Exploded later_exploded = { 2015, 1, -1, 1, 12, 34, 56, 0 };
base::Time::Exploded later_exploded = {2015, 1, 5, 1, 12, 34, 56, 0};
base::Time later = base::Time::FromUTCExploded(later_exploded);
const struct {