[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20260910205657.GK23438@brightrain.aerifal.cx>
Date: Thu, 10 Sep 2026 16:56:58 -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, Sep 09, 2026 at 08:00:33PM -0400, Rich Felker wrote:
> 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.
See attached which is probably what I'll go with.
View attachment "0001-vf-w-scanf-fix-integer-overflow-in-mlc-allocation-wi.patch" of type "text/plain" (1637 bytes)
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.
