[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20260805191340.GU27423@brightrain.aerifal.cx>
Date: Wed, 5 Aug 2026 15:13:40 -0400
From: Rich Felker <dalias@...c.org>
To: Matthias Goergens <matthias.goergens@...il.com>
Cc: musl@...ts.openwall.com
Subject: Re: [PATCH 1/2] stdio: avoid field width overflow in scanf
On Wed, Aug 05, 2026 at 03:52:14PM +0800, Matthias Goergens wrote:
> Decimal field widths are accumulated in int. The old expression can
> overflow as early as valid width 2147483600 because it adds the digit
> character value before subtracting '0'. Larger widths overflow too.
>
> This issue was noted in a 2018 stdio review, but the report described an
> INT_MIN case and the reachable positive boundary remained unclear.
>
> Use the existing bounded-parser idiom from printf, with the digit
> subtraction grouped before addition. Accept widths through INT_MAX and
> reject larger values as an invalid format rather than allowing them to
> wrap.
> ---
> src/stdio/vfscanf.c | 4 +++-
> src/stdio/vfwscanf.c | 4 +++-
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/src/stdio/vfscanf.c b/src/stdio/vfscanf.c
> index b78a374d..07bb3845 100644
> --- a/src/stdio/vfscanf.c
> +++ b/src/stdio/vfscanf.c
> @@ -118,7 +118,9 @@ int vfscanf(FILE *restrict f, const char *restrict fmt, va_list ap)
> }
>
> for (width=0; isdigit(*p); p++) {
> - width = 10*width + *p - '0';
> + if (width > INT_MAX/10U || *p-'0' > INT_MAX-10*width)
> + goto fmt_fail;
> + width = 10*width + (*p-'0');
> }
>
> if (*p=='m') {
> diff --git a/src/stdio/vfwscanf.c b/src/stdio/vfwscanf.c
> index 82f48604..1497aa0d 100644
> --- a/src/stdio/vfwscanf.c
> +++ b/src/stdio/vfwscanf.c
> @@ -141,7 +141,9 @@ int vfwscanf(FILE *restrict f, const wchar_t *restrict fmt, va_list ap)
> }
>
> for (width=0; iswdigit(*p); p++) {
> - width = 10*width + *p - '0';
> + if (width > INT_MAX/10U || *p-'0' > INT_MAX-10*width)
> + goto fmt_fail;
> + width = 10*width + (*p-'0');
> }
>
> if (*p=='m') {
I don't think this is a correct fix. For printf, we get to bail out
with an error because excess width will necessarily produce output
characters and overflow the int return value, which is specified as an
error (POSIX) or is UB (plain C).
But in scanf the width is a limit on input, and there is no inherent
error if its excessive.
So either before or after this patch, we're still doing something
wrong when trying to interpret a width > INT_MAX. And we only get in
the situation to begin with in the case where the format string in the
source has a ridiculous width to begin with.
If we're going to do something here I'd like to do it right rather
than just swap one bad behavior for another. But I don't see any
really good fix, since for example a width of 2^33 could be valid and
successfully read a numeric field prefixed with that many redundant
zeros. So it seems like actually interpreting large widths correctly
requires being able to represent the exact number, not just cap them
out or treat them as "unlimited". But this doesn't seem like
functionality any real program would/should need, which makes it hard
to justify spending effort fixing it..
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.
