Re: Bug in Newlib's snprintf implementation

[Jeffrey Walton <noloader at gmail dot com>]

On Fri, Feb 23, 2018 at 4:43 AM, Corinna Vinschen <vinschen@redhat.com> wrote:
> On Feb 23 04:34, Sebastian Huber wrote:
>> ----- Am 23. Feb 2018 um 4:00 schrieb Dominik Maier domenukk@gmail.com:
>>
>> > Hello,
>> >
>> > I've already reported this over at MinGW but since I could also
>> > reproduce the Bug in Cygwin and understand both use Newlib, I figured
>> > I'd post this here, too.
>> >
>> > The snprintf implementation is not compliant. This can lead to bugs and
>> > memory corruptions.
>> >
>> > A simple PoC is the following
>> >
>> >    char buf[4];
>> >    snprintf(buf, 4,"te%s", "st");
>> >    printf("%s", buf);
>> >
>> > It should print "tes" (0-terminated) but instad prints "test" and
>> > appends random memory (tested in cygwin and mingw).
>>
>> I cannot reproduce this issue on RTEMS. Her buf is "tes\0" and the return value is 4.
>
> Tested on most recent Cygwin 2.10.0, x86 and x86_64.  Both return
> "tes\0" and the return value is 4.
>
>> > Then len field in this case is 4, which is correct (apart from the
>> > missing zero termination), however it will return an error (-1) for any
>> > larger input.
>>
>> It should return the actual size of the produced string and not -1.
>
> Slight correction: snprintf returns the size of the string it would
> have produced if it hadn't truncated the output.  So a size value
> of 4 ("test" would have been the desired result) is correct.

I don't believe this is correct. The string of 'test' and the NULL
requires 5, not 4. 4 indicates truncation.

According to the man page (https://linux.die.net/man/3/snprintf):

<SNIP>
RETURN VALUE

Upon successful return, these functions return the number of
characters printed (excluding the null byte used to end output to
strings).

The functions snprintf() and vsnprintf() do not write more than size
bytes (including the terminating null byte ('\0')). If the output was
truncated due to this limit then the return value is the number of
characters (excluding the terminating null byte) which would have been
written to the final string if enough space had been available. Thus,
a return value of size or more means that the output was truncated.
(See also below under NOTES.)
</SNIP>

I cringe when I have to audit code that uses the non-safer versions of
the functions. This is what I look for:

    size_t N = ...; // n is some constexpr
    char buff [N];
    int ret = snprintf(buff, sizeof(buff), <remainder of expression>);

    if (ret == -1)
        // failed ....

    if (ret >= N)
        // trucation ...

My apologies if I have misunderstood things.

Jeff