[PROPOSED 0/5] Fix some unlikely undefined behaviors
This patchest fixes some bugs in localtime.c when TZif files specify a UT offset equal to -2**31. Such an offset overflows when negated on a 32-bit platform, and this leads to undefined behavior – typically wraparound arithmetic, but the C standard says anything can happen. The patchset also changes zic.c to reject attempts to create such an offset, adjusts some nearby code to be a bit cleaner and more portable. Paul Eggert (5): Omit unnecessary L suffixes zic no longer generates utoff == -2**31 Make sure 2**31 - 1 is signed Port TWOS_COMPLEMENT to signed-magnitude hosts Disallow UT offsets equal to -2**31 NEWS | 4 ++++ localtime.c | 48 ++++++++++++++++++++++++++++++++---------------- private.h | 7 +++++-- strftime.c | 5 +++-- zic.c | 7 ++++--- 5 files changed, 48 insertions(+), 23 deletions(-) -- 2.51.0
* zic.c (addtype): Omit unnecessary L suffix in decimal integer constants that need not be of type ‘long’. --- localtime.c | 2 +- zic.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/localtime.c b/localtime.c index b80a3413..355822d2 100644 --- a/localtime.c +++ b/localtime.c @@ -2191,7 +2191,7 @@ localsub(struct state const *sp, time_t const *timep, int_fast32_t setname, ** To get (wrong) behavior that's compatible with System V Release 2.0 ** you'd replace the statement below with ** t += ttisp->tt_utoff; - ** timesub(&t, 0L, sp, tmp); + ** timesub(&t, 0, sp, tmp); */ result = timesub(&t, ttisp->tt_utoff, sp, tmp); if (result) { diff --git a/zic.c b/zic.c index a7c70895..eecd8ac2 100644 --- a/zic.c +++ b/zic.c @@ -3760,7 +3760,7 @@ addtype(zic_t utoff, char const *abbr, bool isdst, bool ttisstd, bool ttisut) { register int i, j; - if (! (-1L - 2147483647L <= utoff && utoff <= 2147483647L)) { + if (! (-1 - 2147483647 <= utoff && utoff <= 2147483647)) { error(_("UT offset out of range")); exit(EXIT_FAILURE); } -- 2.51.0
This rejects outlandish zic input like "Zone X -596523:14:08 - XXT". * NEWS: Mention this. * zic.c (addtype): Do not allow a UT offset of -2**31. --- NEWS | 2 ++ zic.c | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index f645aafa..b4f3ead8 100644 --- a/NEWS +++ b/NEWS @@ -26,6 +26,8 @@ Unreleased, experimental changes zic no longer assumes you can fflush a read-only stream. (Problem reported by Christos Zoulas.) + zic no longer generates UT offsets equal to -2**31, as RFC 9636 + prohibits them. Release 2025c - 2025-12-10 14:42:37 -0800 diff --git a/zic.c b/zic.c index eecd8ac2..b69414dd 100644 --- a/zic.c +++ b/zic.c @@ -3760,7 +3760,8 @@ addtype(zic_t utoff, char const *abbr, bool isdst, bool ttisstd, bool ttisut) { register int i, j; - if (! (-1 - 2147483647 <= utoff && utoff <= 2147483647)) { + /* RFC 9636 section 3.2 specifies this range for utoff. */ + if (! (-2147483647 <= utoff && utoff <= 2147483647)) { error(_("UT offset out of range")); exit(EXIT_FAILURE); } -- 2.51.0
Fix a porting bug in strftime.c, triggered on theoretical (but conforming) hosts where 0x7fffffff is of type unsigned long. While we’re at it, give 2**31 - 1 a name to make this problem less likely in the future, and use the name consistently. * private.h (TWO_31_MINUS_1): New macro. * strftime.c (MKTIME_FITS_IN): * zic.c (ZIC32_MIN, ZIC32_MAX, addtype): Use it. --- private.h | 5 ++++- strftime.c | 5 +++-- zic.c | 6 +++--- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/private.h b/private.h index 55a8d548..7e1e350a 100644 --- a/private.h +++ b/private.h @@ -1083,7 +1083,10 @@ char *asctime_r(struct tm const *restrict, char *restrict); char *ctime_r(time_t const *, char *); #endif /* HAVE_INCOMPATIBLE_CTIME_R */ -/* Handy macros that are independent of tzfile implementation. */ +/* Handy constants that are independent of tzfile implementation. */ + +/* 2**31 - 1 as a signed integer, and usable in #if. */ +#define TWO_31_MINUS_1 2147483647 enum { SECSPERMIN = 60, diff --git a/strftime.c b/strftime.c index 487a5234..c2490105 100644 --- a/strftime.c +++ b/strftime.c @@ -49,8 +49,9 @@ and account for the tm_year origin (1900) and time_t origin (1970). */ #define MKTIME_FITS_IN(min, max) \ ((min) < 0 \ - && ((min) + 0x7fffffff) / 366 / 24 / 60 / 60 / 2 + 1970 - 1900 < INT_MIN \ - && INT_MAX < ((max) - 0x7fffffff) / 366 / 24 / 60 / 60 / 2 + 1970 - 1900) + && (((min) + TWO_31_MINUS_1) / 366 / 24 / 60 / 60 / 2 + 1970 - 1900 \ + < INT_MIN) \ + && INT_MAX < ((max) - TWO_31_MINUS_1) / 366 / 24 / 60 / 60 / 2 + 1970 - 1900) /* MKTIME_MIGHT_OVERFLOW is true if mktime can fail due to time_t overflow or if it is not known whether mktime can fail, diff --git a/zic.c b/zic.c index b69414dd..d50ad012 100644 --- a/zic.c +++ b/zic.c @@ -31,8 +31,8 @@ typedef int_fast64_t zic_t; static zic_t const ZIC_MIN = INT_FAST64_MIN, ZIC_MAX = INT_FAST64_MAX, - ZIC32_MIN = -1 - (zic_t) 0x7fffffff, - ZIC32_MAX = 0x7fffffff; + ZIC32_MIN = -1 - (zic_t) TWO_31_MINUS_1, + ZIC32_MAX = TWO_31_MINUS_1; #define SCNdZIC SCNdFAST64 #ifndef ZIC_MAX_ABBR_LEN_WO_WARN @@ -3761,7 +3761,7 @@ addtype(zic_t utoff, char const *abbr, bool isdst, bool ttisstd, bool ttisut) register int i, j; /* RFC 9636 section 3.2 specifies this range for utoff. */ - if (! (-2147483647 <= utoff && utoff <= 2147483647)) { + if (! (-TWO_31_MINUS_1 <= utoff && utoff <= TWO_31_MINUS_1)) { error(_("UT offset out of range")); exit(EXIT_FAILURE); } -- 2.51.0
* private.h (TWOS_COMPLEMENT): Port to signed-magnitude, which is still used on Unisys Clearpath MCP (a C90 platform). --- private.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/private.h b/private.h index 7e1e350a..42a1c892 100644 --- a/private.h +++ b/private.h @@ -943,7 +943,7 @@ ATTRIBUTE_PURE time_t time2posix_z(timezone_t, time_t); #define TYPE_BIT(type) (CHAR_BIT * (ptrdiff_t) sizeof(type)) #define TYPE_SIGNED(type) (((type) -1) < 0) -#define TWOS_COMPLEMENT(t) ((t) ~ (t) 0 < 0) +#define TWOS_COMPLEMENT(type) (TYPE_SIGNED (type) && (! ~ (type) -1)) /* Minimum and maximum of two values. Use lower case to avoid naming clashes with standard include files. */ -- 2.51.0
* NEWS: Mention this. * localtime.c (int_fast32_2s): New type. (detzcode): Return it instead of returning int_fast32_t, and do not silently pretend that -2**31 is -2**31 + 1 on platforms that are not two’s complement. All callers changed. (tzloadbody): Reject TZif files containing a UT offset of -2**31. Internet RFC 9636 prohibits that, and it can cause undefined behavior both here and in callers. --- NEWS | 6 ++++-- localtime.c | 46 +++++++++++++++++++++++++++++++--------------- 2 files changed, 35 insertions(+), 17 deletions(-) diff --git a/NEWS b/NEWS index b4f3ead8..30f757df 100644 --- a/NEWS +++ b/NEWS @@ -26,8 +26,10 @@ Unreleased, experimental changes zic no longer assumes you can fflush a read-only stream. (Problem reported by Christos Zoulas.) - zic no longer generates UT offsets equal to -2**31, as RFC 9636 - prohibits them. + zic no longer generates UT offsets equal to -2**31 and localtime.c + no longer accepts them, as they can cause trouble in both + localtime.c and its callers. RFC 9636 prohibits such offsets. + Release 2025c - 2025-12-10 14:42:37 -0800 diff --git a/localtime.c b/localtime.c index 355822d2..a506f097 100644 --- a/localtime.c +++ b/localtime.c @@ -496,6 +496,16 @@ typedef ptrdiff_t desigidx_type; # error "TZNAME_MAXIMUM too large" #endif +/* A type that can represent any 32-bit two's complement integer, + i.e., any integer in the range -2**31 .. 2**31 - 1. + Ordinarily this is int_fast32_t, but on non-C23 hosts + that are not two's complement it is int_fast64_t. */ +#if INT_FAST32_MIN < -TWO_31_MINUS_1 +typedef int_fast32_t int_fast32_2s; +#else +typedef int_fast64_t int_fast32_2s; +#endif + struct ttinfo { /* time type information */ int_least32_t tt_utoff; /* UT offset in seconds */ desigidx_type tt_desigidx; /* abbreviation list index */ @@ -638,15 +648,14 @@ ttunspecified(struct state const *sp, int i) return memcmp(abbr, UNSPEC, sizeof UNSPEC) == 0; } -static int_fast32_t +static int_fast32_2s detzcode(const char *const codep) { - register int_fast32_t result; register int i; - int_fast32_t one = 1; - int_fast32_t halfmaxval = one << (32 - 2); - int_fast32_t maxval = halfmaxval - 1 + halfmaxval; - int_fast32_t minval = -1 - maxval; + int_fast32_2s + maxval = TWO_31_MINUS_1, + minval = -1 - maxval, + result; result = codep[0] & 0x7f; for (i = 1; i < 4; ++i) @@ -654,8 +663,7 @@ detzcode(const char *const codep) if (codep[0] & 0x80) { /* Do two's-complement negation even on non-two's-complement machines. - If the result would be minval - 1, return minval. */ - result -= !TWOS_COMPLEMENT(int_fast32_t) && result != 0; + This cannot overflow, as int_fast32_2s is wide enough. */ result += minval; } return result; @@ -1033,14 +1041,15 @@ tzloadbody(char const *name, struct state *sp, char tzloadflags, char version = up->tzhead.tzh_version[0]; bool skip_datablock = stored == 4 && version; int_fast32_t datablock_size; - int_fast32_t ttisstdcnt = detzcode(up->tzhead.tzh_ttisstdcnt); - int_fast32_t ttisutcnt = detzcode(up->tzhead.tzh_ttisutcnt); int_fast64_t prevtr = -1; int_fast32_t prevcorr; - int_fast32_t leapcnt = detzcode(up->tzhead.tzh_leapcnt); - int_fast32_t timecnt = detzcode(up->tzhead.tzh_timecnt); - int_fast32_t typecnt = detzcode(up->tzhead.tzh_typecnt); - int_fast32_t charcnt = detzcode(up->tzhead.tzh_charcnt); + int_fast32_2s + ttisstdcnt = detzcode(up->tzhead.tzh_ttisstdcnt), + ttisutcnt = detzcode(up->tzhead.tzh_ttisutcnt), + leapcnt = detzcode(up->tzhead.tzh_leapcnt), + timecnt = detzcode(up->tzhead.tzh_timecnt), + typecnt = detzcode(up->tzhead.tzh_typecnt), + charcnt = detzcode(up->tzhead.tzh_charcnt); char const *p = up->buf + tzheadsize; /* Although tzfile(5) currently requires typecnt to be nonzero, support future formats that may allow zero typecnt @@ -1109,9 +1118,16 @@ tzloadbody(char const *name, struct state *sp, char tzloadflags, for (i = 0; i < sp->typecnt; ++i) { register struct ttinfo * ttisp; unsigned char isdst, desigidx; + int_fast32_2s utoff = detzcode(p); + + /* Reject a UT offset equal to -2**31, as it might + cause trouble both in this file and in callers. + Also, it violates RFC 9636 section 3.2. */ + if (utoff < -TWO_31_MINUS_1) + return EINVAL; ttisp = &sp->ttis[i]; - ttisp->tt_utoff = detzcode(p); + ttisp->tt_utoff = utoff; p += 4; isdst = *p++; if (! (isdst < 2)) -- 2.51.0
participants (1)
-
Paul Eggert