|
|
Message-ID: <20260910000033.GJ23438@brightrain.aerifal.cx>
Date: Wed, 9 Sep 2026 20:00:33 -0400
From: Rich Felker <dalias@...c.org>
To: Matthias Goergens <matthias.goergens@...il.com>
Cc: musl@...ts.openwall.com
Subject: Re: [PATCH 2/2] stdio: check allocated scanf buffer sizes
On Wed, Aug 05, 2026 at 03:52:15PM +0800, Matthias Goergens wrote:
> The scanf allocation modifier computes initial and grown buffer sizes
> without checking that element counts and byte sizes fit in size_t. On
> 32-bit targets, %1073741823mlc wraps a wide allocation to zero before
> the first input character is stored.
>
> Reject unrepresentable initial wide buffers. Bound all narrow and wide
> geometric growth before updating element counts. Report ENOMEM and use
> the existing allocation-failure cleanup path.
> ---
> src/stdio/vfscanf.c | 13 +++++++++++++
> src/stdio/vfwscanf.c | 16 ++++++++++++++++
> 2 files changed, 29 insertions(+)
>
> diff --git a/src/stdio/vfscanf.c b/src/stdio/vfscanf.c
> index 07bb3845..4e06e494 100644
> --- a/src/stdio/vfscanf.c
> +++ b/src/stdio/vfscanf.c
> @@ -6,6 +6,7 @@
> #include <limits.h>
> #include <string.h>
> #include <stdint.h>
> +#include <errno.h>
>
> #include "stdio_impl.h"
> #include "shgetc.h"
> @@ -228,6 +229,10 @@ int vfscanf(FILE *restrict f, const char *restrict fmt, va_list ap)
> k = t=='c' ? width+1U : 31;
> if (size == SIZE_l) {
> if (alloc) {
> + if (k > SIZE_MAX/sizeof(wchar_t)) {
> + errno = ENOMEM;
> + goto alloc_fail;
> + }
> wcs = malloc(k*sizeof(wchar_t));
This indeed is a missing check where underallocation could happen if
the passed-in width is enormous.
> if (!wcs) goto alloc_fail;
> } else {
> @@ -243,6 +248,10 @@ int vfscanf(FILE *restrict f, const char *restrict fmt, va_list ap)
> }
> if (wcs) wcs[i++] = wc;
> if (alloc && i==k) {
> + if (k > (SIZE_MAX/sizeof(wchar_t)-1)/2) {
> + errno = ENOMEM;
> + goto alloc_fail;
> + }
> k+=k+1;
> wchar_t *tmp = realloc(wcs, k*sizeof(wchar_t));
> if (!tmp) goto alloc_fail;
> @@ -256,6 +265,10 @@ int vfscanf(FILE *restrict f, const char *restrict fmt, va_list ap)
> while (scanset[(c=shgetc(f))+1]) {
> s[i++] = c;
> if (i==k) {
> + if (k > (SIZE_MAX-1)/2) {
> + errno = ENOMEM;
> + goto alloc_fail;
> + }
> k+=k+1;
> char *tmp = realloc(s, k);
> if (!tmp) goto alloc_fail;
The increment k+=k+1 was chosen such that, if k was initially the
dimension of an existant array, the arithmetic cannot overflow. The
initial k can be at most PTRDIFF_MAX==(SIZE_MAX-1)/2, in which case
2*k+1 is SIZE_MAX. So no further check is needed.
> diff --git a/src/stdio/vfwscanf.c b/src/stdio/vfwscanf.c
> index 1497aa0d..c1a87b74 100644
> --- a/src/stdio/vfwscanf.c
> +++ b/src/stdio/vfwscanf.c
> @@ -6,6 +6,8 @@
> #include <wctype.h>
> #include <limits.h>
> #include <string.h>
> +#include <stdint.h>
> +#include <errno.h>
>
> #include "stdio_impl.h"
> #include "shgetc.h"
> @@ -245,8 +247,14 @@ int vfwscanf(FILE *restrict f, const wchar_t *restrict fmt, va_list ap)
>
> i = 0;
> if (alloc) {
> + s = 0;
> + wcs = 0;
> k = t=='c' ? width+1U : 31;
> if (size == SIZE_l) {
> + if (k > SIZE_MAX/sizeof(wchar_t)) {
> + errno = ENOMEM;
> + goto alloc_fail;
> + }
> wcs = malloc(k*sizeof(wchar_t));
> if (!wcs) goto alloc_fail;
> } else {
> @@ -261,6 +269,10 @@ int vfwscanf(FILE *restrict f, const wchar_t *restrict fmt, va_list ap)
> if (wcs) {
> wcs[i++] = c;
> if (alloc && i==k) {
> + if (k > (SIZE_MAX/sizeof(wchar_t)-1)/2) {
> + errno = ENOMEM;
> + goto alloc_fail;
> + }
> k += k+1;
> wchar_t *tmp = realloc(wcs, k*sizeof(wchar_t));
> if (!tmp) goto alloc_fail;
> @@ -271,6 +283,10 @@ int vfwscanf(FILE *restrict f, const wchar_t *restrict fmt, va_list ap)
> if (l<0) goto input_fail;
> i += l;
> if (alloc && i > k-4) {
> + if (k > (SIZE_MAX-1)/2) {
> + errno = ENOMEM;
> + goto alloc_fail;
> + }
> k += k+1;
> char *tmp = realloc(s, k);
> if (!tmp) goto alloc_fail;
> --
> 2.55.0
Likewise for these.
Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.