[PATCH v2] libio: Fix crash in fputws [BZ #20632]

[Peter Ammon]

> On Sep 8, 2024, at 8:34 PM, Peter Ammon <corydoras@ridiculousfish.com> wrote:
> 
> This fixes a buffer overflow in wide character string output, reproducing
> when output fails, such as if the output fd is closed or is redirected
> to a full device.
> 
> Wide character output data attempts to maintain the invariant that
> `_IO_buf_base <= _IO_write_base <= _IO_write_end <= _IO_buf_end` (that is,
> that the write region is a sub-region of `_IO_buf`). Prior to this commit,
> this invariant is violated by the `_IO_wfile_overflow` function as so:
> 
> 1. `_IO_wsetg` is called, assigning `_IO_write_base` to `_IO_buf_base`
> 2. `_IO_doallocbuf` is called, which jumps to `_IO_wfile_doallocate` via
>    the _IO_wfile_jumps vtable. This function then assigns the wide data
>    `_IO_buf_base` and `_IO_buf_end` to a malloc'd buffer.
> 
> Thus the invariant is violated. The fix is simply to reverse the order:
> malloc the `_IO_buf` first and then assign `_IO_write_base` to it.
> 
> We also take this opportunity to defensively guard the initialization of
> the number of unwritten characters via pointer arithmetic. We now check
> that the buffer end is not before the buffer beginning; this matches a
> similar defensive check in the narrow analogue `fileops.c`.
> 
> Add a test which fails without the fix.
> 
> Signed-off-by: Peter Ammon <corydoras@ridiculousfish.com>
> ---
> libio/Makefile                      |  2 +
> libio/test-fputs-unbuffered-full.c  | 78 +++++++++++++++++++++++++++++
> libio/test-fputws-unbuffered-full.c | 21 ++++++++
> libio/wfileops.c                    | 10 ++--
> 4 files changed, 107 insertions(+), 4 deletions(-)
> create mode 100644 libio/test-fputs-unbuffered-full.c
> create mode 100644 libio/test-fputws-unbuffered-full.c
> 
> diff --git a/libio/Makefile b/libio/Makefile
> index 59f3ee0b7c..f8adfb8951 100644
> --- a/libio/Makefile
> +++ b/libio/Makefile
> @@ -86,6 +86,8 @@ tests = \
>   bug-wmemstream1 \
>   bug-wsetpos \
>   test-fmemopen \
> +  test-fputs-unbuffered-full \
> +  test-fputws-unbuffered-full \
>   tst-atime \
>   tst-bz22415 \
>   tst-bz24051 \
> diff --git a/libio/test-fputs-unbuffered-full.c b/libio/test-fputs-unbuffered-full.c
> new file mode 100644
> index 0000000000..8fb762f701
> --- /dev/null
> +++ b/libio/test-fputs-unbuffered-full.c
> @@ -0,0 +1,78 @@
> +/* Regression test for 20632.
> +   Copyright (C) 2024 Free Software Foundation, Inc.
> +   Copyright The GNU Toolchain Authors.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <errno.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <support/check.h>
> +#include <support/xunistd.h>
> +#include <unistd.h>
> +
> +#ifndef WIDE
> +# define TEST_NAME "fputs-unbuffered-full"
> +# define CHAR char
> +# define FPUTS fputs
> +# define TEXT "0123456789ABCDEF"
> +#else
> +# include <wchar.h>
> +# define TEST_NAME "fputws-unbuffered-full"
> +# define CHAR wchar_t
> +# define FPUTS fputws
> +# define TEXT L"0123456789ABCDEF"
> +#endif /* WIDE */
> +
> +
> +static int
> +do_test (void)
> +{
> +  /* Open an unbuffered stream to /dev/full. */
> +  FILE *fp = fopen ("/dev/full", "w");
> +  TEST_VERIFY_EXIT (fp != NULL);
> +  int ret = setvbuf (fp, NULL, _IONBF, 0);
> +  TEST_VERIFY_EXIT (ret == 0);
> +
> +  /* Output a long string. */
> +  const int sz = 4096;
> +  CHAR *buff = calloc (sz+1, sizeof *buff);
> +  for (int i=0; i < sz; i++)
> +    buff[i] = (CHAR) 'x';
> +  buff[sz] = (CHAR) '\0';
> +  errno = 0;
> +  ret = FPUTS (buff, fp);
> +  TEST_VERIFY (ret == EOF);
> +  TEST_VERIFY (errno == ENOSPC);
> +  free (buff);
> +
> +  /* Output shorter strings. */
> +  for (int i=0; i < 1024; i++)
> +    {
> +      errno = 0;
> +      ret = FPUTS (TEXT, fp);
> +      TEST_VERIFY (ret == EOF);
> +      TEST_VERIFY (errno == ENOSPC);
> +
> +      /* Call malloc, triggering a crash if its
> +         function pointers have been overwritten. */
> +      void *volatile ptr = malloc (1);
> +      free (ptr);
> +    }
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/libio/test-fputws-unbuffered-full.c b/libio/test-fputws-unbuffered-full.c
> new file mode 100644
> index 0000000000..f3d79326e4
> --- /dev/null
> +++ b/libio/test-fputws-unbuffered-full.c
> @@ -0,0 +1,21 @@
> +/* Regression test for 20632.
> +   Copyright (C) 2024 Free Software Foundation, Inc.
> +   Copyright The GNU Toolchain Authors.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#define WIDE 1
> +#include "./test-fputs-unbuffered-full.c"
> diff --git a/libio/wfileops.c b/libio/wfileops.c
> index 6de5968358..8a1912cc0e 100644
> --- a/libio/wfileops.c
> +++ b/libio/wfileops.c
> @@ -420,14 +420,14 @@ _IO_wfile_overflow (FILE *f, wint_t wch)
> 	{
> 	  _IO_wdoallocbuf (f);
> 	  _IO_free_wbackup_area (f);
> -	  _IO_wsetg (f, f->_wide_data->_IO_buf_base,
> -		     f->_wide_data->_IO_buf_base, f->_wide_data->_IO_buf_base);
> 
> 	  if (f->_IO_write_base == NULL)
> 	    {
> 	      _IO_doallocbuf (f);
> 	      _IO_setg (f, f->_IO_buf_base, f->_IO_buf_base, f->_IO_buf_base);
> 	    }
> +	  _IO_wsetg (f, f->_wide_data->_IO_buf_base,
> +		     f->_wide_data->_IO_buf_base, f->_wide_data->_IO_buf_base);
> 	}
>       else
> 	{
> @@ -958,7 +958,7 @@ _IO_wfile_xsputn (FILE *f, const void *data, size_t n)
>   const wchar_t *s = (const wchar_t *) data;
>   size_t to_do = n;
>   int must_flush = 0;
> -  size_t count;
> +  size_t count = 0;
> 
>   if (n <= 0)
>     return 0;
> @@ -967,7 +967,6 @@ _IO_wfile_xsputn (FILE *f, const void *data, size_t n)
>      (or the filebuf is unbuffered), use sys_write directly. */
> 
>   /* First figure out how much space is available in the buffer. */
> -  count = f->_wide_data->_IO_write_end - f->_wide_data->_IO_write_ptr;
>   if ((f->_flags & _IO_LINE_BUF) && (f->_flags & _IO_CURRENTLY_PUTTING))
>     {
>       count = f->_wide_data->_IO_buf_end - f->_wide_data->_IO_write_ptr;
> @@ -985,6 +984,9 @@ _IO_wfile_xsputn (FILE *f, const void *data, size_t n)
> 	    }
> 	}
>     }
> +  else if (f->_wide_data->_IO_write_end > f->_wide_data->_IO_write_ptr)
> +    count = f->_wide_data->_IO_write_end - f->_wide_data->_IO_write_ptr; /* Space available. */
> +
>   /* Then fill the buffer. */
>   if (count > 0)
>     {
> --
> 2.46.0

Ping, thank you!