Avoid conversion overflow from struct tm.
See discussion in
https://boringssl-review.googlesource.com/c/boringssl/+/65967/comment/4b0fb2a6_78bfab3a/
struct tm is defined with tm_ values as all ints.
Conversion inside this code is all bounds checked around valid
times and can't overflow, but because struct tm uses 1 based months
and 1900 based years, we need to modify the input values when
converting a struct tm.
Rather than do awkward bounds checks, just accept an int64_t on
input and we don't have to care.
OPENSSL_gmtime_adj gains checks to ensure the cumulative days
and seconds values passed in may not overflow.
While we are here we also ensure that OPENSSL_gmtime_adj does
not modify the output struct tm in the failure case.
Also while we are here, just make the offset_seconds value of
OPENSSL_gmtime_adj an int64_t - because long was never the
correct type.
Add tests for all this.
Change-Id: I40ac019c4274b5388c97736cf85cede951d8b7ae
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66047
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: Bob Beck <bbe@google.com>
diff --git a/crypto/asn1/asn1_test.cc b/crypto/asn1/asn1_test.cc
index 76d6889..314ac3d 100644
--- a/crypto/asn1/asn1_test.cc
+++ b/crypto/asn1/asn1_test.cc
@@ -1162,8 +1162,8 @@
struct tm tm1, tm2;
int days, secs;
- OPENSSL_posix_to_tm(0, &tm1);
- OPENSSL_posix_to_tm(0, &tm2);
+ EXPECT_TRUE(OPENSSL_posix_to_tm(0, &tm1));
+ EXPECT_TRUE(OPENSSL_posix_to_tm(0, &tm2));
// Test values that are too large and should be rejected.
EXPECT_FALSE(OPENSSL_gmtime_adj(&tm1, INT_MIN, INT_MIN));
EXPECT_FALSE(OPENSSL_gmtime_adj(&tm1, INT_MAX, INT_MAX));
@@ -2475,6 +2475,87 @@
#endif
}
+static auto TimeToTuple(const tm &t) {
+ return std::make_tuple(t.tm_year, t.tm_mon, t.tm_mday, t.tm_hour, t.tm_min,
+ t.tm_sec);
+}
+
+TEST(ASN1Test, TimeOverflow) {
+ // Input time is out of range and may overflow internal calculations to shift
+ // |tm_year| and |tm_mon| to a more normal value.
+ tm overflow_year = {};
+ overflow_year.tm_year = INT_MAX - 1899;
+ overflow_year.tm_mday = 1;
+ tm overflow_month = {};
+ overflow_month.tm_mon = INT_MAX;
+ overflow_month.tm_mday = 1;
+ int64_t posix_u64;
+ EXPECT_FALSE(OPENSSL_tm_to_posix(&overflow_year, &posix_u64));
+ EXPECT_FALSE(OPENSSL_tm_to_posix(&overflow_month, &posix_u64));
+ time_t posix;
+ EXPECT_FALSE(OPENSSL_timegm(&overflow_year, &posix));
+ EXPECT_FALSE(OPENSSL_timegm(&overflow_month, &posix));
+ EXPECT_FALSE(
+ OPENSSL_gmtime_adj(&overflow_year, /*offset_day=*/0, /*offset_sec=*/0));
+ EXPECT_FALSE(
+ OPENSSL_gmtime_adj(&overflow_month, /*offset_day=*/0, /*offset_sec=*/0));
+ int days, secs;
+ EXPECT_FALSE(
+ OPENSSL_gmtime_diff(&days, &secs, &overflow_year, &overflow_year));
+ EXPECT_FALSE(
+ OPENSSL_gmtime_diff(&days, &secs, &overflow_month, &overflow_month));
+
+ // Input time is in range, but even adding one second puts it out of range.
+ tm max_time = {};
+ max_time.tm_year = 9999 - 1900;
+ max_time.tm_mon = 12 - 1;
+ max_time.tm_mday = 31;
+ max_time.tm_hour = 23;
+ max_time.tm_min = 59;
+ max_time.tm_sec = 59;
+ tm copy = max_time;
+ EXPECT_TRUE(OPENSSL_gmtime_adj(©, /*offset_day=*/0, /*offset_sec=*/0));
+ EXPECT_EQ(TimeToTuple(copy), TimeToTuple(max_time));
+ EXPECT_FALSE(OPENSSL_gmtime_adj(©, /*offset_day=*/0, /*offset_sec=*/1));
+
+ // Likewise for the earliest representable time.
+ tm min_time = {};
+ min_time.tm_year = 0 - 1900;
+ min_time.tm_mon = 1 - 1;
+ min_time.tm_mday = 1;
+ min_time.tm_hour = 0;
+ min_time.tm_min = 0;
+ min_time.tm_sec = 0;
+ copy = min_time;
+ EXPECT_TRUE(OPENSSL_gmtime_adj(©, /*offset_day=*/0, /*offset_sec=*/0));
+ EXPECT_EQ(TimeToTuple(copy), TimeToTuple(min_time));
+ EXPECT_FALSE(OPENSSL_gmtime_adj(©, /*offset_day=*/0, /*offset_sec=*/-1));
+
+ // Test we can offset between the minimum and maximum times.
+ const int64_t kValidTimeRange = 315569519999;
+ copy = min_time;
+ EXPECT_TRUE(OPENSSL_gmtime_adj(©, /*offset_day=*/0, kValidTimeRange));
+ EXPECT_EQ(TimeToTuple(copy), TimeToTuple(max_time));
+ EXPECT_TRUE(OPENSSL_gmtime_adj(©, /*offset_day=*/0, -kValidTimeRange));
+ EXPECT_EQ(TimeToTuple(copy), TimeToTuple(min_time));
+
+ // The second offset may even exceed kValidTimeRange if it is canceled out by
+ // offset_day.
+ EXPECT_TRUE(OPENSSL_gmtime_adj(©, /*offset_day=*/-1,
+ kValidTimeRange + 24 * 3600));
+ EXPECT_EQ(TimeToTuple(copy), TimeToTuple(max_time));
+ EXPECT_TRUE(OPENSSL_gmtime_adj(©, /*offset_day=*/1,
+ -kValidTimeRange - 24 * 3600));
+ EXPECT_EQ(TimeToTuple(copy), TimeToTuple(min_time));
+
+ // Make sure the internal calculations for |OPENSSL_gmtime_adj| stay in
+ // bounds.
+ copy = max_time;
+ EXPECT_FALSE(OPENSSL_gmtime_adj(©, INT_MAX, LONG_MAX));
+ copy = min_time;
+ EXPECT_FALSE(OPENSSL_gmtime_adj(©, INT_MIN, LONG_MIN));
+}
+
// The ASN.1 macros do not work on Windows shared library builds, where usage of
// |OPENSSL_EXPORT| is a bit stricter.
#if !defined(OPENSSL_WINDOWS) || !defined(BORINGSSL_SHARED_LIBRARY)
diff --git a/crypto/asn1/internal.h b/crypto/asn1/internal.h
index 57e6966..601a3ca 100644
--- a/crypto/asn1/internal.h
+++ b/crypto/asn1/internal.h
@@ -87,7 +87,7 @@
// must be in the range of year 0000 to 9999 both before and after the update or
// a failure will be returned.
OPENSSL_EXPORT int OPENSSL_gmtime_adj(struct tm *tm, int offset_day,
- long offset_sec);
+ int64_t offset_sec);
// OPENSSL_gmtime_diff calculates the difference between |from| and |to|. It
// returns one, and outputs the difference as a number of days and seconds in
diff --git a/crypto/asn1/posix_time.c b/crypto/asn1/posix_time.c
index 2422bd3..ff95cca 100644
--- a/crypto/asn1/posix_time.c
+++ b/crypto/asn1/posix_time.c
@@ -26,12 +26,12 @@
#include "internal.h"
#define SECS_PER_HOUR (60 * 60)
-#define SECS_PER_DAY (24 * SECS_PER_HOUR)
+#define SECS_PER_DAY (INT64_C(24) * SECS_PER_HOUR)
// Is a year/month/day combination valid, in the range from year 0000
// to 9999?
-static int is_valid_date(int year, int month, int day) {
+static int is_valid_date(int64_t year, int64_t month, int64_t day) {
if (day < 1 || month < 1 || year < 0 || year > 9999) {
return 0;
}
@@ -62,7 +62,7 @@
// Is a time valid? Leap seconds of 60 are not considered valid, as
// the POSIX time in seconds does not include them.
-static int is_valid_time(int hours, int minutes, int seconds) {
+static int is_valid_time(int64_t hours, int64_t minutes, int64_t seconds) {
if (hours < 0 || minutes < 0 || seconds < 0 || hours > 23 || minutes > 59 ||
seconds > 59) {
return 0;
@@ -70,17 +70,22 @@
return 1;
}
-// Is a int64 time representing a time within our expected range?
-static int is_valid_epoch_time(int64_t time) {
- // 0000-01-01 00:00:00 UTC to 9999-12-31 23:59:59 UTC
- return (int64_t)-62167219200 <= time && time <= (int64_t)253402300799;
+// 0000-01-01 00:00:00 UTC
+#define MIN_POSIX_TIME INT64_C(-62167219200)
+// 9999-12-31 23:59:59 UTC
+#define MAX_POSIX_TIME INT64_C(253402300799)
+
+// Is an int64 time within our expected range?
+static int is_valid_posix_time(int64_t time) {
+ return MIN_POSIX_TIME <= time && time <= MAX_POSIX_TIME;
}
// Inspired by algorithms presented in
// https://howardhinnant.github.io/date_algorithms.html
// (Public Domain)
-static int posix_time_from_utc(int year, int month, int day, int hours,
- int minutes, int seconds, int64_t *out_time) {
+static int posix_time_from_utc(int64_t year, int64_t month, int64_t day,
+ int64_t hours, int64_t minutes, int64_t seconds,
+ int64_t *out_time) {
if (!is_valid_date(year, month, day) ||
!is_valid_time(hours, minutes, seconds)) {
return 0;
@@ -108,7 +113,7 @@
static int utc_from_posix_time(int64_t time, int *out_year, int *out_month,
int *out_day, int *out_hours, int *out_minutes,
int *out_seconds) {
- if (!is_valid_epoch_time(time)) {
+ if (!is_valid_posix_time(time)) {
return 0;
}
int64_t days = time / SECS_PER_DAY;
@@ -143,24 +148,21 @@
}
int OPENSSL_tm_to_posix(const struct tm *tm, int64_t *out) {
- // Ensure the additions below do not overflow.
- if (tm->tm_year > 9999 || tm->tm_mon > 12) {
- return 0;
- }
-
- return posix_time_from_utc(tm->tm_year + 1900, tm->tm_mon + 1, tm->tm_mday,
- tm->tm_hour, tm->tm_min, tm->tm_sec, out);
+ return posix_time_from_utc(tm->tm_year + INT64_C(1900),
+ tm->tm_mon + INT64_C(1), tm->tm_mday, tm->tm_hour,
+ tm->tm_min, tm->tm_sec, out);
}
int OPENSSL_posix_to_tm(int64_t time, struct tm *out_tm) {
- memset(out_tm, 0, sizeof(struct tm));
- if (!utc_from_posix_time(time, &out_tm->tm_year, &out_tm->tm_mon,
- &out_tm->tm_mday, &out_tm->tm_hour, &out_tm->tm_min,
- &out_tm->tm_sec)) {
+ struct tm tmp_tm = {0};
+ if (!utc_from_posix_time(time, &tmp_tm.tm_year, &tmp_tm.tm_mon,
+ &tmp_tm.tm_mday, &tmp_tm.tm_hour, &tmp_tm.tm_min,
+ &tmp_tm.tm_sec)) {
return 0;
}
- out_tm->tm_year -= 1900;
- out_tm->tm_mon -= 1;
+ tmp_tm.tm_year -= 1900;
+ tmp_tm.tm_mon -= 1;
+ *out_tm = tmp_tm;
return 1;
}
@@ -192,43 +194,47 @@
return out_tm;
}
-int OPENSSL_gmtime_adj(struct tm *tm, int off_day, long offset_sec) {
+int OPENSSL_gmtime_adj(struct tm *tm, int offset_day, int64_t offset_sec) {
int64_t posix_time;
- if (!posix_time_from_utc(tm->tm_year + 1900, tm->tm_mon + 1, tm->tm_mday,
- tm->tm_hour, tm->tm_min, tm->tm_sec, &posix_time)) {
+ if (!OPENSSL_tm_to_posix(tm, &posix_time)) {
return 0;
}
- if (!utc_from_posix_time(
- posix_time + (int64_t)off_day * SECS_PER_DAY + offset_sec,
- &tm->tm_year, &tm->tm_mon, &tm->tm_mday, &tm->tm_hour, &tm->tm_min,
- &tm->tm_sec)) {
+ static_assert(INT_MAX <= INT64_MAX / SECS_PER_DAY,
+ "day offset in seconds cannot overflow");
+ static_assert(MAX_POSIX_TIME <= INT64_MAX - INT_MAX * SECS_PER_DAY,
+ "addition cannot overflow");
+ static_assert(MIN_POSIX_TIME >= INT64_MIN - INT_MIN * SECS_PER_DAY,
+ "addition cannot underflow");
+ posix_time += offset_day * SECS_PER_DAY;
+ if (posix_time > 0 && offset_sec > INT64_MAX - posix_time) {
return 0;
}
- tm->tm_year -= 1900;
- tm->tm_mon -= 1;
+ if (posix_time < 0 && offset_sec < INT64_MIN - posix_time) {
+ return 0;
+ }
+ posix_time += offset_sec;
+
+ if (!OPENSSL_posix_to_tm(posix_time, tm)) {
+ return 0;
+ }
return 1;
}
int OPENSSL_gmtime_diff(int *out_days, int *out_secs, const struct tm *from,
const struct tm *to) {
- int64_t time_to;
- if (!posix_time_from_utc(to->tm_year + 1900, to->tm_mon + 1, to->tm_mday,
- to->tm_hour, to->tm_min, to->tm_sec, &time_to)) {
+ int64_t time_to, time_from;
+ if (!OPENSSL_tm_to_posix(to, &time_to) ||
+ !OPENSSL_tm_to_posix(from, &time_from)) {
return 0;
}
- int64_t time_from;
- if (!posix_time_from_utc(from->tm_year + 1900, from->tm_mon + 1,
- from->tm_mday, from->tm_hour, from->tm_min,
- from->tm_sec, &time_from)) {
- return 0;
- }
+ // Times are in range, so these calculations can not overflow.
+ static_assert(SECS_PER_DAY <= INT_MAX, "seconds per day does not fit in int");
+ static_assert((MAX_POSIX_TIME - MIN_POSIX_TIME) / SECS_PER_DAY <= INT_MAX,
+ "range of valid POSIX times, in days, does not fit in int");
int64_t timediff = time_to - time_from;
int64_t daydiff = timediff / SECS_PER_DAY;
timediff %= SECS_PER_DAY;
- if (daydiff > INT_MAX || daydiff < INT_MIN) {
- return 0;
- }
*out_secs = (int)timediff;
*out_days = (int)daydiff;
return 1;
