[PROPOSED] Prefer <stdckdint.h> for overflow checking
If <stdckdint.h> is available, or if its macros can be implemented easily, use the macros for overflow checking. This should help avoid future integer overflow bugs. * date.c (dogmt, timeout): * localtime.c (localsub, timesub, increment_overflow) (increment_overflow32, increment_overflow_time, time2sub): * zdump.c (sumsize, tzalloc): * zic.c (size_sum, size_product, grow_nitems_alloc, oadd, tadd): Prefer ckd_add etc. to doing overflow checking by hand. * private.h (HAVE_STDCKDINT_H): Default to true if __has_include(<stdckdint.h>). Include <stdckdint.h> if available. (ckd_add, ckd_sub, ckd_mul): Provide substitutes on recent-enough GCC-compatible compilers. --- Makefile | 2 ++ NEWS | 5 ++++- date.c | 10 ++++++++++ localtime.c | 33 +++++++++++++++++++++++++++++++++ private.h | 29 +++++++++++++++++++++++++++++ zdump.c | 12 ++++++++++++ zic.c | 31 +++++++++++++++++++++++++++++++ 7 files changed, 121 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 5ec0d4a..92d78cb 100644 --- a/Makefile +++ b/Makefile @@ -230,6 +230,8 @@ LDLIBS= # functions like 'link' or variables like 'tzname' required by POSIX # -DHAVE_SETENV=0 if your system lacks the setenv function # -DHAVE_SNPRINTF=0 if your system lacks the snprintf function +# -DHAVE_STDCKDINT_H=0 if neither <stdckdint.h> nor substitutes like +# __builtin_add_overflow work* # -DHAVE_STDINT_H=0 if <stdint.h> does not work* # -DHAVE_STRFTIME_L if <time.h> declares locale_t and strftime_l # -DHAVE_STRDUP=0 if your system lacks the strdup function diff --git a/NEWS b/NEWS index 7c46d3c..a401bee 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,7 @@ News for the tz database Fix some pre-1996 timestamps in northern Canada. C89 is now deprecated; please use C99 or later. Portability fixes for AIX, libintl, MS-Windows, musl, z/OS + In C code, use more C23 features if available. C23 timegm now supported by default Fixes for unlikely integer overflows @@ -55,7 +56,9 @@ Unreleased, experimental changes uninitialized data has undefined behavior (strftime problem reported by Robert Elz). - Check more carefully for unlikely integer overflows. + Check more carefully for unlikely integer overflows, preferring + C23 <stdckdint.h> to overflow checking by hand, as the latter has + had obscure bugs. Changes to build procedure diff --git a/date.c b/date.c index cbc0ec1..1319462 100644 --- a/date.c +++ b/date.c @@ -122,8 +122,14 @@ dogmt(void) for (n = 0; environ[n] != NULL; ++n) continue; +#if defined ckd_add && defined ckd_mul + if (!ckd_add(&n, n, 2) && !ckd_mul(&n, n, sizeof *fakeenv) + && n <= SIZE_MAX) + fakeenv = malloc(n); +#else if (n <= min(PTRDIFF_MAX, SIZE_MAX) / sizeof *fakeenv - 2) fakeenv = malloc((n + 2) * sizeof *fakeenv); +#endif if (fakeenv == NULL) { fprintf(stderr, _("date: Memory exhausted\n")); errensure(); @@ -187,8 +193,12 @@ timeout(FILE *fp, char const *format, struct tm const *tmp) ptrdiff_t size = 1024 / 2; for ( ; ; ) { +#ifdef ckd_mul + bool bigger = !ckd_mul(&size, size, 2) && size <= SIZE_MAX; +#else bool bigger = (size <= min(PTRDIFF_MAX, SIZE_MAX) / 2 && (size *= 2, true)); +#endif char *newcp = bigger ? realloc(cp, size) : NULL; if (!newcp) { fprintf(stderr, diff --git a/localtime.c b/localtime.c index 7326567..7faa5d3 100644 --- a/localtime.c +++ b/localtime.c @@ -1565,6 +1565,14 @@ localsub(struct state const *sp, time_t const *timep, int_fast32_t setname, return NULL; /* "cannot happen" */ result = localsub(sp, &newt, setname, tmp); if (result) { +#if defined ckd_add && defined ckd_sub + if (t < sp->ats[0] + ? ckd_sub(&result->tm_year, + result->tm_year, years) + : ckd_add(&result->tm_year, + result->tm_year, years)) + return NULL; +#else register int_fast64_t newy; newy = result->tm_year; @@ -1574,6 +1582,7 @@ localsub(struct state const *sp, time_t const *timep, int_fast32_t setname, if (! (INT_MIN <= newy && newy <= INT_MAX)) return NULL; result->tm_year = newy; +#endif } return result; } @@ -1783,6 +1792,12 @@ timesub(const time_t *timep, int_fast32_t offset, y = newy; } +#ifdef ckd_add + if (ckd_add(&tmp->tm_year, y, -TM_YEAR_BASE)) { + errno = EOVERFLOW; + return NULL; + } +#else if (!TYPE_SIGNED(time_t) && y < TM_YEAR_BASE) { int signed_y = y; tmp->tm_year = signed_y - TM_YEAR_BASE; @@ -1793,6 +1808,7 @@ timesub(const time_t *timep, int_fast32_t offset, errno = EOVERFLOW; return NULL; } +#endif tmp->tm_yday = idays; /* ** The "extra" mods below avoid overflow problems. @@ -1867,6 +1883,9 @@ ctime_r(const time_t *timep, char *buf) static bool increment_overflow(int *ip, int j) { +#ifdef ckd_add + return ckd_add(ip, *ip, j); +#else register int const i = *ip; /* @@ -1879,22 +1898,30 @@ increment_overflow(int *ip, int j) return true; *ip += j; return false; +#endif } static bool increment_overflow32(int_fast32_t *const lp, int const m) { +#ifdef ckd_add + return ckd_add(lp, *lp, m); +#else register int_fast32_t const l = *lp; if ((l >= 0) ? (m > INT_FAST32_MAX - l) : (m < INT_FAST32_MIN - l)) return true; *lp += m; return false; +#endif } static bool increment_overflow_time(time_t *tp, int_fast32_t j) { +#ifdef ckd_add + return ckd_add(tp, *tp, j); +#else /* ** This is like ** 'if (! (TIME_T_MIN <= *tp + j && *tp + j <= TIME_T_MAX)) ...', @@ -1906,6 +1933,7 @@ increment_overflow_time(time_t *tp, int_fast32_t j) return true; *tp += j; return false; +#endif } static bool @@ -2029,11 +2057,16 @@ time2sub(struct tm *const tmp, return WRONG; } } +#ifdef ckd_add + if (ckd_add(&yourtm.tm_year, y, -TM_YEAR_BASE)) + return WRONG; +#else if (increment_overflow32(&y, -TM_YEAR_BASE)) return WRONG; if (! (INT_MIN <= y && y <= INT_MAX)) return WRONG; yourtm.tm_year = y; +#endif if (yourtm.tm_sec >= 0 && yourtm.tm_sec < SECSPERMIN) saved_seconds = 0; else if (yourtm.tm_year < EPOCH_YEAR - TM_YEAR_BASE) { diff --git a/private.h b/private.h index 4315a85..4d40582 100644 --- a/private.h +++ b/private.h @@ -392,6 +392,35 @@ typedef unsigned long uintmax_t; # define SIZE_MAX ((size_t) -1) #endif +/* Support ckd_add, ckd_sub, ckd_mul on C23 or recent-enough GCC-like + hosts, unless compiled with -DHAVE_STDCKDINT_H=0 or with pre-C23 EDG. */ +#if !defined HAVE_STDCKDINT_H && defined __has_include +# if __has_include(<stdckdint.h>) +# define HAVE_STDCKDINT_H true +# endif +#endif +#ifdef HAVE_STDCKDINT_H +# if HAVE_STDCKDINT_H +# include <stdckdint.h> +# endif +#elif defined __EDG__ +/* Do nothing, to work around EDG bug <https://bugs.gnu.org/53256>. */ +#elif defined __has_builtin +# if __has_builtin(__builtin_add_overflow) +# define ckd_add(r, a, b) __builtin_add_overflow(a, b, r) +# endif +# if __has_builtin(__builtin_sub_overflow) +# define ckd_sub(r, a, b) __builtin_sub_overflow(a, b, r) +# endif +# if __has_builtin(__builtin_mul_overflow) +# define ckd_mul(r, a, b) __builtin_mul_overflow(a, b, r) +# endif +#elif 7 <= __GNUC__ +# define ckd_add(r, a, b) __builtin_add_overflow(a, b, r) +# define ckd_sub(r, a, b) __builtin_sub_overflow(a, b, r) +# define ckd_mul(r, a, b) __builtin_mul_overflow(a, b, r) +#endif + #if 3 <= __GNUC__ # define ATTRIBUTE_CONST __attribute__((const)) # define ATTRIBUTE_MALLOC __attribute__((__malloc__)) diff --git a/zdump.c b/zdump.c index 669d8fd..89bcd25 100644 --- a/zdump.c +++ b/zdump.c @@ -137,9 +137,15 @@ size_overflow(void) static ATTRIBUTE_PURE ptrdiff_t sumsize(size_t a, size_t b) { +#ifdef ckd_add + ptrdiff_t sum; + if (!ckd_add(&sum, a, b) && sum <= SIZE_MAX) + return sum; +#else ptrdiff_t sum_max = min(PTRDIFF_MAX, SIZE_MAX); if (a <= sum_max && b <= sum_max - a) return a + b; +#endif size_overflow(); } @@ -254,9 +260,15 @@ tzalloc(char const *val) ptrdiff_t initial_nenvptrs = 1; /* Counting the trailing NULL pointer. */ while (*e++) { +# ifdef ckd_add + if (ckd_add(&initial_nenvptrs, initial_envptrs, 1) + || SIZE_MAX < initial_envptrs) + size_overflow(); +# else if (initial_nenvptrs == min(PTRDIFF_MAX, SIZE_MAX) / sizeof *environ) size_overflow(); initial_nenvptrs++; +# endif } fakeenv0size = sumsize(valsize, valsize); fakeenv0size = max(fakeenv0size, 64); diff --git a/zic.c b/zic.c index a69d4e0..8f8088e 100644 --- a/zic.c +++ b/zic.c @@ -475,18 +475,30 @@ size_overflow(void) static ATTRIBUTE_PURE ptrdiff_t size_sum(size_t a, size_t b) { +#ifdef ckd_add + ptrdiff_t sum; + if (!ckd_add(&sum, a, b) && sum <= SIZE_MAX) + return sum; +#else ptrdiff_t sum_max = min(PTRDIFF_MAX, SIZE_MAX); if (a <= sum_max && b <= sum_max - a) return a + b; +#endif size_overflow(); } static ATTRIBUTE_PURE ptrdiff_t size_product(ptrdiff_t nitems, ptrdiff_t itemsize) { +#ifdef ckd_mul + ptrdiff_t product; + if (!ckd_mul(&product, nitems, itemsize) && product <= SIZE_MAX) + return product; +#else ptrdiff_t nitems_max = min(PTRDIFF_MAX, SIZE_MAX) / itemsize; if (nitems <= nitems_max) return nitems * itemsize; +#endif size_overflow(); } @@ -536,11 +548,18 @@ static ptrdiff_t grow_nitems_alloc(ptrdiff_t *nitems_alloc, ptrdiff_t itemsize) { ptrdiff_t addend = (*nitems_alloc >> 1) + 1; +#if defined ckd_add && defined ckd_mul + ptrdiff_t product; + if (!ckd_add(nitems_alloc, *nitems_alloc, addend) + && !ckd_mul(&product, *nitems_alloc, itemsize) && product <= SIZE_MAX) + return product; +#else ptrdiff_t amax = min(PTRDIFF_MAX, SIZE_MAX); if (*nitems_alloc <= ((amax - 1) / 3 * 2) / itemsize) { *nitems_alloc += addend; return *nitems_alloc * itemsize; } +#endif memory_exhausted(_("integer overflow")); } @@ -3716,16 +3735,28 @@ time_overflow(void) static ATTRIBUTE_PURE zic_t oadd(zic_t t1, zic_t t2) { +#ifdef ckd_add + zic_t sum; + if (!ckd_add(&sum, t1, t2)) + return sum; +#else if (t1 < 0 ? ZIC_MIN - t1 <= t2 : t2 <= ZIC_MAX - t1) return t1 + t2; +#endif time_overflow(); } static ATTRIBUTE_PURE zic_t tadd(zic_t t1, zic_t t2) { +#ifdef ckd_add + zic_t sum; + if (!ckd_add(&sum, t1, t2) && min_time <= sum && sum <= max_time) + return sum; +#else if (t1 < 0 ? min_time - t1 <= t2 : t2 <= max_time - t1) return t1 + t2; +#endif if (t1 == min_time || t1 == max_time) return t1; time_overflow(); -- 2.38.1
participants (1)
-
Paul Eggert