[PATCH v3] stdio-common: Fix heap overflow in scanf %mc pattern [BZ #34008]

[Carlos O'Donell]

On 4/2/26 1:23 PM, Rocket Ma wrote:
> * stdio-common/vfscanf-internal.c: when `WIDTH` in `%WIDTHmc` or
> `%WIDTHmC` greater than 1024, user could read one more byte into heap,
> leading into off-by-one overflow.

Please don't worry about using a GNU Changelog style description.

Please write a commit message that explains what is being changed
and why to support future developers looking at the change.

CVE-2026-5450 has been reserved for this issue, but has not yet
been published. Please feel free to reference it in the commit
message.

> This patch fixes Bug 34008[1].

Drop. Your first line already describes this.

> 
> [1]: https://sourceware.org/bugzilla/show_bug.cgi?id=34008

Drop. Not needed.

> 
> Use support/check.h for regression test

Drop. Not normally described either.

> 
> Signed-off-by: Rocket Ma <marocketbd@gmail.com>
> ---
>   stdio-common/Makefile               |  1 +
>   stdio-common/bug-vfscanf-internal.c | 59 +++++++++++++++++++++++++++++
>   stdio-common/vfscanf-internal.c     |  6 +--
>   3 files changed, 63 insertions(+), 3 deletions(-)
>   create mode 100644 stdio-common/bug-vfscanf-internal.c
> 
> diff --git a/stdio-common/Makefile b/stdio-common/Makefile
> index 210944837e..ec3fff246b 100644
> --- a/stdio-common/Makefile
> +++ b/stdio-common/Makefile
> @@ -200,6 +200,7 @@ aux := \
>   
>   tests := \
>     bug-vfprintf-nargs \
> +  bug-vfscanf-internal \
>     bug1 \
>     bug3 \
>     bug4 \
> diff --git a/stdio-common/bug-vfscanf-internal.c b/stdio-common/bug-vfscanf-internal.c
> new file mode 100644
> index 0000000000..54ab192e50
> --- /dev/null
> +++ b/stdio-common/bug-vfscanf-internal.c
> @@ -0,0 +1,59 @@

Please add an LGPLv2+ license header like the other tests, with a one line description.

> +#include <stddef.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <wchar.h>
> +#include <stdlib.h>
> +#include <malloc.h>
> +#include <support/check.h>
> +
> +static size_t
> +get_cookie (size_t *chunk)
> +{
> +/* heap related test need to ignore out-of-bound access */
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Warray-bounds"
> +  return chunk[-1] & ~1; /* ignore prev_inuse bit */
> +#pragma GCC diagnostic pop
> +}
> +
> +#if __SIZEOF_POINTER__ == 8
> +#  define WIDTH 0x409
> +#  define SCANFSTR "%1033mc"
> +#else /* 32bit target? */
> +#  define WIDTH 0x40d
> +#  define SCANFSTR "%1037mc"
> +#endif
> +#define CHUNKSZ 0x410
> +static int
> +do_test (void)
> +{
> +  char *input = malloc (WIDTH + 1);
> +  TEST_VERIFY (input != NULL);
> +  memset (input, 'A', WIDTH);
> +  input[WIDTH] = '\0';
> +
> +  /* THIS TEST UNIT REQUIRE SOME HEAP LAYOUT! Which is related to
> +   current ptmalloc, when malloc is updated, this test may be inaccurate.
> +   Anyway, we should construct a case where we allocate a chunk just
> +   lower than scanf-alloced chunk to detect if heap overflow happens. */

As Andreas points out this should use mcheck() to look for the overflow.

Would you be able to look at how the mtrace tests are executed? I think that
just running it under the normal mtrace infrastructure should trigger checking.

e.g.
579 tst-ungetc-leak-ENV = \
580   MALLOC_TRACE=$(objpfx)tst-ungetc-leak.mtrace \
581   LD_PRELOAD=$(common-objpfx)malloc/libc_malloc_debug.so

Test is added to tests, and out file to tests-special, and generated for
out and mtrace that need cleanup, and test *-ENV with special values.

> +  void *hole = malloc (WIDTH - 1);
> +  size_t *guard = malloc (0x20);
> +  if ((size_t) hole + CHUNKSZ != (size_t) guard)
> +    {
> +      puts ("Unexpected heap layout: \"guard\" is not adjacent to \"hole\"");
> +      return 77;
> +    }
> +  size_t cookie = get_cookie (guard);
> +  free (hole);
> +
> +  char *buf = NULL;
> +  TEST_VERIFY (sscanf (input, SCANFSTR, &buf) != -1);
> +  TEST_VERIFY (buf != NULL);
> +  TEST_VERIFY (get_cookie (guard) == cookie);
> +
> +  free (buf);
> +  free (input);
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/stdio-common/vfscanf-internal.c b/stdio-common/vfscanf-internal.c
> index 59fc8208aa..b33ee0652c 100644
> --- a/stdio-common/vfscanf-internal.c
> +++ b/stdio-common/vfscanf-internal.c
> @@ -856,7 +856,7 @@ __vfscanf_internal (FILE *s, const char *format, va_list argptr,
>   			  /* Enlarge the buffer.  */
>   			  size_t newsize
>   			    = strsize
> -			      + (strsize >= width ? width - 1 : strsize);
> +			      + (strsize >= width ? width : strsize);

May you please make this ">" to match the other code blocks?

Either we grow as "2*strsize" or we have optimized and grown only by the remaining width
left to read.

The ">=" here makes the logic more difficult than it needs to be.

>   
>   			  str = (char *) realloc (*strptr, newsize);
>   			  if (str == NULL)
> @@ -929,7 +929,7 @@ __vfscanf_internal (FILE *s, const char *format, va_list argptr,
>   		      && wstr == (wchar_t *) *strptr + strsize)
>   		    {
>   		      size_t newsize
> -			= strsize + (strsize > width ? width - 1 : strsize);
> +			= strsize + (strsize > width ? width : strsize);
>   		      /* Enlarge the buffer.  */
>   		      wstr = (wchar_t *) realloc (*strptr,
>   						  newsize * sizeof (wchar_t));
> @@ -984,7 +984,7 @@ __vfscanf_internal (FILE *s, const char *format, va_list argptr,
>   		    && wstr == (wchar_t *) *strptr + strsize)
>   		  {
>   		    size_t newsize
> -		      = strsize + (strsize > width ? width - 1 : strsize);
> +		      = strsize + (strsize > width ? width : strsize);
>   		    /* Enlarge the buffer.  */
>   		    wstr = (wchar_t *) realloc (*strptr,
>   						newsize * sizeof (wchar_t));


-- 
Cheers,
Carlos.