Fix integer overflow in OPENSSL_gmtime_adj OpenSSL uses integer parameters for this function, and the multiplication here ends up being done as an integer. Since we support values up to year 9999, it is possible for someone to pass in a number of days to the "adj" function to adjust a base time far enough to overflow a 32 bit integer. Change-Id: Iedfc33d8bf90d70049f99897df1d193fb29805d0 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55125 Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: Bob Beck <bbe@google.com>
diff --git a/crypto/asn1/asn1_test.cc b/crypto/asn1/asn1_test.cc index 2f0ec76..7d9688b 100644 --- a/crypto/asn1/asn1_test.cc +++ b/crypto/asn1/asn1_test.cc
@@ -1047,6 +1047,32 @@ } } +TEST(ASN1Test, AdjTime) { + struct tm tm1, tm2; + int days, secs; + + OPENSSL_posix_to_tm(0, &tm1); + 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)); + // Basic functionality. + EXPECT_TRUE(OPENSSL_gmtime_adj(&tm2, 1, 1)); + EXPECT_TRUE(OPENSSL_gmtime_diff(&days, &secs, &tm1, &tm2)); + EXPECT_EQ(days, 1); + EXPECT_EQ(secs, 1); + EXPECT_TRUE(OPENSSL_gmtime_diff(&days, &secs, &tm2, &tm1)); + EXPECT_EQ(days, -1); + EXPECT_EQ(secs, -1); + // Test a value of days that is very large, but valid. + EXPECT_TRUE(OPENSSL_gmtime_adj(&tm2, 2932800, 0)); + EXPECT_TRUE(OPENSSL_gmtime_diff(&days, &secs, &tm1, &tm2)); + EXPECT_EQ(days, 2932801); + EXPECT_EQ(secs, 1); + EXPECT_TRUE(OPENSSL_gmtime_diff(&days, &secs, &tm2, &tm1)); + EXPECT_EQ(days, -2932801); + EXPECT_EQ(secs, -1); +} static std::vector<uint8_t> StringToVector(const std::string &str) { return std::vector<uint8_t>(str.begin(), str.end()); }
diff --git a/crypto/asn1/internal.h b/crypto/asn1/internal.h index 882b5fb..4f98a1a 100644 --- a/crypto/asn1/internal.h +++ b/crypto/asn1/internal.h
@@ -96,7 +96,8 @@ // |offset_day| days and |offset_sec| seconds. It returns zero on failure. |tm| // must be in the range of year 0000 to 9999 both before and after the update or // a failure will be returned. -int OPENSSL_gmtime_adj(struct tm *tm, int offset_day, long offset_sec); +OPENSSL_EXPORT int OPENSSL_gmtime_adj(struct tm *tm, int offset_day, + long 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 81fbe83..7dd9e5b 100644 --- a/crypto/asn1/posix_time.c +++ b/crypto/asn1/posix_time.c
@@ -191,9 +191,10 @@ tm->tm_hour, tm->tm_min, tm->tm_sec, &posix_time)) { return 0; } - if (!utc_from_posix_time(posix_time + 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 (!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)) { return 0; } tm->tm_year -= 1900;