Re: [PATCH] scanf.3: Do not mention the ERANGE error

[Zack Weinberg <zack@owlfolio.org>]

Alejandro Colomar <alx.manpages@gmail.com> writes:
> On 12/9/22 22:41, Zack Weinberg via Libc-alpha wrote:
>> The `scanf` functions have undefined behavior if numeric input
>> overflows.  This means it is *impossible* to detect malformed input
>> reliably using these functions.
>> Many input specifications (e.g. `%s`, `%[^\n]`) read a sequence of
>> characters into a destination buffer whose size is unspecified; any
>> use of such specifications renders `scanf` every bit as dangerous as
>> `gets`.
…
>> Best practice is not to use any of these functions at all.
…
> I'm inclined to add that in that manual page.  Is there anything that
> can be saved from that page, or should we burn it all?  To be more
> specific:
>
> -  Are there any functions in that page that are still useful for any
>    corner cases, or are they all useless?
> -  Are there any conversion specifiers that can be used safely?

Hmm, this turns out to be a bit of a rabbit hole.

There are two major design-level problems with the scanf family.
The more important one is that string input conversions (%s, %[…])
will read an unlimited number of characters by default, oblivious to
the size of the destination buffer — exactly the same design flaw as
‘gets’.  They do stop scanning at _any_ whitespace (not just \n) so,
if you’re trying to craft exploit code, there are more byte values that
must be avoided, but this is only a minor obstacle.  They can, however,
be used safely, either by supplying a field width that accurately
reflects the size of the destination buffer, or by using the ‘m’
modifier (a POSIX extension), which directs scanf to allocate the
right amount of space for the string with malloc.

(Field widths are awkward to use because you have to write them as
decimal constants _inside the format string_, which makes them more
likely to get out of sync with the actual size of the buffer than,
say, the buffer-size argument to ‘fgets’, but this is not a fatal
flaw in and of itself.)

The other design-level issue affects all of the numeric conversions:
if the result of (abstract, infinite-precision) numeric input conversion
does not fit in the variable supplied to hold the result of that conversion,
the behavior is undefined.  The manpage says that you get an ERANGE error
in this case, and that may be the behavior _glibc_ guarantees (I don’t
actually know for sure), but in the modern era of compilers drawing
inferences from undefined behavior, a guarantee by one C library is
not good enough.

That covers everything except %c and %n, which are safe but somewhat
pointless in isolation.

> Or the converse questions:
>
> -  Which conversion specifiers (or modifiers) are impossible to use
>    safely as gets(3) and should therefore be marked as deprecated in
>    the manual page (and probably warned in GCC)?
> -  Which functions in that page are impossible to use safely and
>    should therefore be marked as deprecated?
>
> Would you please mark them as [[deprecated]] in glibc too?

I don’t think glibc should unilaterally deprecate any function that’s
specified by ISO C.  And, the scanf family *can* be used safely with
sufficient care — read entire lines of input with getline, then split
them up into fields with sscanf using only %ms and %m[…], and finally
parse all numeric fields by hand with strtoX — the issue is more that,
if you limit yourself to the set of scanf operations that are 100% safe,
you’re left only with stuff that is arguably *easier* to do with <string.h>
and <regex.h> functions.

In a more sober tone of voice I suggest this text for the manpage:

.SH BUGS
By default, the 
.IR %s " and " %[
conversions will read an
.I unlimited
number of characters from the input.
In this mode they are just as unsafe as the infamous
.BR gets (3).
One should always specify either a field width,
or the
.I m
modifier,
with every use of
.IR %s " or " %[ .
.PP
If a numeric input conversion produces a value
that is not representable in the type of the corresponding argument
(e.g. if 99999 is to be stored in an
.IR "unsigned short" ),
ISO C says that the behavior is undefined.
The GNU C Library guarantees to treat this condition as a
.IR "matching failure" ,
but portable code should avoid using the numeric conversions.

I also suggest that GCC should add diagnostics to -Wformat and/or
-Wformat-security to catch use of %s and %[ with no size specified;
if glibc doesn’t already treat numeric overflow as a matching failure,
it should be changed to do so; and maybe someone should write up a
proposal for the C standard to make the same change.

zw