Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Sun, 3 Dec 2023 16:38:55 -0500
From: Rich Felker <>
To: Morten Welinder <>
Subject: Re: stdio/vfprintf.c

On Sun, Dec 03, 2023 at 11:28:43AM -0500, Morten Welinder wrote:
> Looking at
> I see a few issues:
> 1. If "i=-1" in getint on line 424 is reached and there are more
> digits then the next overflow check will itself overflow in
> "INT_MAX-10*i"

No, the evaluation of INT_MAX-10*i is not reached because i >
INT_MAX/10U is true.

> 2. The getint call on line 504 doesn't check for overflow.  If it did,
> getint could just return -1 right away on overflow.

I'm unclear what you think it should check. Precisions too large to be
representable become -1 (no limit), as intended. They will not be
exceeded because attempting to output more than INT_MAX bytes is
already an error which will be caught when reached, before the (lost)
gigantic precision is exceeded.

> 3. The "w=-w;" on line 488 doesn't check for overflow which will
> happen for INT_MIN.

This is probably a bug that should be handled with an explicit check
for INT_MIN and goto overflow.

> 4. The length calculation for "%s" on line 600 implies that strings
> longer than 2G cannot be printed.  It looks deliberate, but is it
> reasonable?

It's an unfortunate contract of the printf family. The return value
must be the number of bytes produced, which is not satisfiable if more
than INT_MAX would be produced. snprintf is explicit that this is a
reportable error, but for the rest, this is probably really undefined

> 5. And speaking of plain "%s" with no width or precision, why is the
> string length even calculated first?  Walking the string twice seems
> inefficient.

Because the code is written as one common path for everything rather
than two special cases for "%s with no width or precision" and "%s
with width or precision".

> 6. This comment and check seems out of date:
> /* This error is only specified for snprintf, but since it's
> * unspecified for other forms, do the same. Stop immediately
> * on overflow; otherwise %n could produce wrong results. */
> if (l > INT_MAX - cnt) goto overflow;
> Since %n allows size modifiers it can already produce wrong results.
> Right right place to check would be at %n handling.

I think you're misreading what "wrong results" means. %n is specified
to store the number into an object of a particular type according to
the modifiers prefixes; this may be a truncated result for %hn or
%hhn, but it's not a wrong result. It's the correct result as

The issue that comment is dealing with is that, in the absence of %n,
it would be perfectly fine for snprintf to "keep going on overflow",
discarding output, as long as the final return value reflects the
overflow error. However, with %n, this "keep going" can have side
effects that are not permitted: an object whose value should not have
been modified will have had a new value stored into it.


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.