[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20260805235423.GV27423@brightrain.aerifal.cx>
Date: Wed, 5 Aug 2026 19:54:23 -0400
From: Rich Felker <dalias@...c.org>
To: Matthias Goergens <matthias.goergens@...il.com>
Cc: musl@...ts.openwall.com
Subject: Re: [PATCH] time: reject overflowing strptime years
On Wed, Aug 05, 2026 at 03:57:26PM +0800, Matthias Goergens wrote:
> The numeric-field loop accumulates directly into int. POSIX.1-2024 gives
> the year inside %F unlimited width, so long input can overflow before
> the result is adjusted to tm_year. Combining %C and %y can overflow too.
>
> Accumulate a checked unsigned magnitude. Perform sign and year
> adjustments in long long, and reject results that cannot fit in the int
> fields of struct tm. Use the same intermediate for century composition.
> ---
> src/time/strptime.c | 24 +++++++++++++++++-------
> 1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/src/time/strptime.c b/src/time/strptime.c
> index 40bb37af..d4440229 100644
> --- a/src/time/strptime.c
> +++ b/src/time/strptime.c
> @@ -2,6 +2,7 @@
> #include <langinfo.h>
> #include <time.h>
> #include <ctype.h>
> +#include <limits.h>
> #include <stddef.h>
> #include <string.h>
> #include <strings.h>
> @@ -10,6 +11,8 @@
> char *strptime(const char *restrict s, const char *restrict f, struct tm *restrict tm)
> {
> int i, w, neg, adj, min, range, *dest, dummy;
> + unsigned x, digit;
> + long long v;
> const char *ex;
> size_t len;
> int want_century = 0, century = 0, relyear = 0;
> @@ -233,10 +236,15 @@ char *strptime(const char *restrict s, const char *restrict f, struct tm *restri
> if (*s == '+') s++;
> else if (*s == '-') neg=1, s++;
> if (!isdigit(*s)) return 0;
> - for (*dest=i=0; i<w && isdigit(*s); i++)
> - *dest = *dest * 10 + *s++ - '0';
> - if (neg) *dest = -*dest;
> - *dest -= adj;
> + for (x=i=0; i<w && isdigit(*s); i++) {
> + digit = *s++ - '0';
> + if (x > (UINT_MAX-digit)/10) return 0;
This is a bad way of doing the range check; it imposes a div per
iteration.
The canonical way to avoid this is something like:
if (x > UINT_MAX/10 || digit > UINT_MAX-10*x)
This way the only div is a constant evaluated at compile-time and the
rest is just a mul that isn't even evaluated until the last stage
before overflow (altho it should be CSE'd with the x*10 below,
anyway):
> + x = x * 10 + digit;
> + }
> + v = neg ? -(long long)x : x;
> + v -= adj;
> + if (v < INT_MIN || v > INT_MAX) return 0;
> + *dest = v;
I wasn't sure about treating this as an error, but I think it matches
the specification:
"Any other conversion specification is executed by scanning
characters until a character matching the next directive is
scanned, or until no more characters can be scanned. These
characters, except the one matching the next directive, are then
compared to the locale values associated with the conversion
specifier. If a match is found, values for the affected tm
structure members are set as specified in the description of the
conversion specification. Case is ignored when matching items in
buf such as month or weekday names. If no match is found,
strptime() fails and no more characters are scanned."
An out-of-representable-range numeric value does seem to meet the
condition "no match is found", and there doesn't seem to be any other
text covering it.
I don't think this was thought about at all when the spec was written,
but it's probably the best we can do with what we have.
> goto update;
> symbolic_range:
> for (i=2*range-1; i>=0; i--) {
> @@ -255,9 +263,11 @@ char *strptime(const char *restrict s, const char *restrict f, struct tm *restri
> }
> }
> if (want_century) {
> - tm->tm_year = relyear;
> - if (want_century & 2) tm->tm_year += century * 100 - 1900;
> - else if (tm->tm_year <= 68) tm->tm_year += 100;
> + v = relyear;
> + if (want_century & 2) v += 100LL * century - 1900;
> + else if (v <= 68) v += 100;
> + if (v < INT_MIN || v > INT_MAX) return 0;
> + tm->tm_year = v;
> }
> return (char *)s;
I think this part looks ok.
Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
