Re: [musl] [PATCH] fix wide printf numbered argument buffer overflow

[Rich Felker <dalias@libc.org>]

On Fri, Apr 14, 2023 at 04:55:42PM +0200, Gabriel Ravier wrote:
> The nl_type and nl_arg arrays defined in vfwprintf may be accessed
> with an index up to and including NL_ARGMAX, but they are only of size
> NL_ARGMAX, meaning they may be written to or read from 1 element too
> far.
> 
> For example, the following program:
> 
>  #include <assert.h>
>  #include <stdio.h>
>  #include <string.h>
>  #include <wchar.h>
> 
> int main(void) {
>    char buffer[500];
>    for (size_t i = 0; i < sizeof(buffer); ++i)
>       buffer[i] = (i % 3) ? 0 : 42;
> 
>    wchar_t result;
> 
>    int ret = swprintf(&result, 1, L"%1$s", "");
>    assert(ret != -1);
> }
> 
> evidences the bug, by sometimes mistakenly failing the assertion
> and always triggering a warning under valgrind (the behavior is highly
> dependent on build configuration - I could only reproduce the assert
> failure with GCC 12.2.1 on a very specific system - but the bug is
> still there, as evidenced by the warning valgrind always emits)

Thanks!

To clarify for readers, the reason it fails is that the wide printf
core checks that the argument type list consists of a contiguous run
of used positions followed by unused positions all the way up to the
end of the buffer. This involves checking the 9 position, which was
out-of-bounds due to an off-by-one error in the declaration. If the
overlapping memory happened to have a nonzero value, it would appear
as a present %9$ slot with one or more prior slots unused, thereby
triggering the error condition.

While all instances of nonconforming behavior and UB are bad, I don't
think this is of particular security relevance, despite being
out-of-bounds array read/write. Write is only reachable if the
application is using format strings with a 9th numbered argument
position, and affected programs seem like they should be
malfunctioning already regardless of any attacker-controlled input.
Still I'm glad to have this fix in time for a release.

Rich