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

Message ID 20260402172336.253214-1-marocketbd@gmail.com (mailing list archive)
State Changes Requested
Headers
Series [v3] stdio-common: Fix heap overflow in scanf %mc pattern [BZ #34008] |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Test passed
redhat-pt-bot/TryBot-32bit success Build for i686

Commit Message

Rocket Ma April 2, 2026, 5:23 p.m. UTC
  * 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.

This patch fixes Bug 34008[1].

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

Use support/check.h for regression test

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
  

Comments

Carlos O'Donell April 2, 2026, 9:51 p.m. UTC | #1
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));
  

Patch

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 @@ 
+#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. */
+  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);
 
 			  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));