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>

crypto/asn1/asn1_test.cc

@@ -1162,8 +1162,8 @@ TEST(ASN1Test, AdjTime) {
   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 @@ TEST(ASN1Test, LargeString) {
 #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(&copy, /*offset_day=*/0, /*offset_sec=*/0));
+  EXPECT_EQ(TimeToTuple(copy), TimeToTuple(max_time));
+  EXPECT_FALSE(OPENSSL_gmtime_adj(&copy, /*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(&copy, /*offset_day=*/0, /*offset_sec=*/0));
+  EXPECT_EQ(TimeToTuple(copy), TimeToTuple(min_time));
+  EXPECT_FALSE(OPENSSL_gmtime_adj(&copy, /*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(&copy, /*offset_day=*/0, kValidTimeRange));
+  EXPECT_EQ(TimeToTuple(copy), TimeToTuple(max_time));
+  EXPECT_TRUE(OPENSSL_gmtime_adj(&copy, /*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(&copy, /*offset_day=*/-1,
+                                 kValidTimeRange + 24 * 3600));
+  EXPECT_EQ(TimeToTuple(copy), TimeToTuple(max_time));
+  EXPECT_TRUE(OPENSSL_gmtime_adj(&copy, /*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(&copy, INT_MAX, LONG_MAX));
+  copy = min_time;
+  EXPECT_FALSE(OPENSSL_gmtime_adj(&copy, 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)

crypto/asn1/internal.h

@@ -87,7 +87,7 @@ OPENSSL_EXPORT int OPENSSL_timegm(const struct tm *tm, time_t *out);
 // 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

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,25 +62,30 @@ static int is_valid_date(int year, int month, int day) {
 
 // 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;
   }
   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 posix_time_from_utc(int year, int month, int day, int hours,
 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 @@ static int utc_from_posix_time(int64_t time, int *out_year, int *out_month,
 }
 
 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 @@ struct tm *OPENSSL_gmtime(const time_t *time, struct tm *out_tm) {
   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;
+  }
+  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;
   }
-  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)) {
+  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;
   }
-  tm->tm_year -= 1900;
-  tm->tm_mon -= 1;
 
   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)) {
-    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)) {
+  int64_t time_to, time_from;
+  if (!OPENSSL_tm_to_posix(to, &time_to) ||
+      !OPENSSL_tm_to_posix(from, &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;