Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
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.