@@ -77,6 +77,20 @@ constrain_int(int x, int min, int max)
}
}
+/** Return true if the addition of a and b would overflow. */
+static inline bool
+time_t_add_overflow(time_t a, time_t b) {
+ static_assert(((time_t) -1) < 0, "OpenVPN assumes time_t is signed");
+ static_assert(((time_t) .9) == 0, "OpenVPN assumes time_t is integer type");
+ static_assert(sizeof(time_t) == sizeof(long) || sizeof(time_t) == sizeof(long long),
+ "OpenVPN assumes that time_t is of type long int or long long int");
+ static const time_t TIME_MAX = sizeof(time_t) == sizeof(long) ?
+ LONG_MAX : LLONG_MAX;
+ static const time_t TIME_MIN = sizeof(time_t) == sizeof(long) ?
+ LONG_MIN : LLONG_MIN;
+ return (a > 0 && b > TIME_MAX - a) || (a < 0 && b < TIME_MIN - a);
+}
+
/*
* Functions used for circular buffer index arithmetic.
*/
@@ -51,7 +51,8 @@ event_timeout_trigger(struct event_timeout *et,
if (et->defined)
{
- int wakeup = (int) et->last + et->n - local_now;
+ ASSERT(!time_t_add_overflow(et->last, et->n));
+ time_t wakeup = et->last + et->n - local_now;
if (wakeup <= 0)
{
#if INTERVAL_DEBUG
@@ -72,7 +73,7 @@ event_timeout_trigger(struct event_timeout *et,
if (tv && wakeup < tv->tv_sec)
{
#if INTERVAL_DEBUG
- dmsg(D_INTERVAL, "EVENT event_timeout_wakeup (%d/%d) etcr=%d", wakeup, et->n, et_const_retry);
+ dmsg(D_INTERVAL, "EVENT event_timeout_wakeup (%ld/%d) etcr=%d", wakeup, et->n, et_const_retry);
#endif
tv->tv_sec = wakeup;
tv->tv_usec = 0;
As reported in trac #922, the wakeup computation in event_timeout_trigger() could overflow. Since time_t and int are signed types, that is officially undefined behvaiour. On systems with a 64-bit signed time_t (most if not all 64-bit system), the overflow was caused by the (unnecessary) cast to "int". Removing that, changing the time of "wakeup" to time_t, and assuming that the system clock on our host system will never be set to the year 292471210579 or later, this can no longer overflow on systems with a 64-bit time_t. For systems with a signed 32-bit time_t however, we need an additional check to prevent signed overflow (and thus undefined behaviour). Since time_t is only specified by C99 to be "an arithmetic type capable of representing times", and no TIME_MAX or TIME_MIN macros are available, checking for overflow is not trivial at all. This patch takes the practical approach, and assumes that time_t is of type "long int" (aka "long") or "long long int" (aka "long long"). POSIX requires time_t to be an "integer type", and all systems I know of use a long int or long long int. To be sure that this assumption holds, this patch uses static asserts. So much for this "simple" overflow check. Trac: #922 Signed-off-by: Steffan Karger <steffan@karger.me> --- src/openvpn/integer.h | 14 ++++++++++++++ src/openvpn/interval.c | 5 +++-- 2 files changed, 17 insertions(+), 2 deletions(-)