[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
* 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
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));
@@ -200,6 +200,7 @@ aux := \
tests := \
bug-vfprintf-nargs \
+ bug-vfscanf-internal \
bug1 \
bug3 \
bug4 \
new file mode 100644
@@ -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>
@@ -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));