[PATCH] svfwscanf: Simplify _sungetwc_r to eliminate apparent buffer overflow

From 2bbbf24fab03472b73296356b7dfb13d616302e9 Mon Sep 17 00:00:00 2001
From: Keith Packard <[email protected]>
Date: Tue, 17 Aug 2021 11:51:52 -0700
Subject: [PATCH] svfwscanf: Simplify _sungetwc_r to eliminate apparent buffer
 overflow

svfwscanf replaces getwc and ungetwc_r. The comments in the code talk
about avoiding file operations, but they also need to bypass the
mbtowc calls as svfwscanf operates on wchar_t, not multibyte data,
which is a more important reason here; they would not work correctly
otherwise.

The ungetwc replacement has code which uses the 3 byte FILE _ubuf
field, but if wchar_t is 32-bits, this field is not large enough to
hold even one wchar_t value. Building in this mode generates warnings
about array overflow:

	In file included from ../../newlib/libc/stdio/svfiwscanf.c:35:
	../../newlib/libc/stdio/vfwscanf.c: In function '_sungetwc_r.isra':
	../../newlib/libc/stdio/vfwscanf.c:316:12: warning: array subscript 4294967295 is above array bounds of 'unsigned char[3]' [-Warray-bounds]
	  316 |   fp->_p = &fp->_ubuf[sizeof (fp->_ubuf) - sizeof (wchar_t)];
	      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
	In file included from ../../newlib/libc/stdio/stdio.h:46,
			 from ../../newlib/libc/stdio/vfwscanf.c:82,
			 from ../../newlib/libc/stdio/svfiwscanf.c:35:
	../../newlib/libc/include/sys/reent.h:216:17: note: while referencing '_ubuf'
	  216 |   unsigned char _ubuf[3]; /* guarantee an ungetc() buffer */
	      |                 ^~~~~

However, the vfwscanf code *never* ungets data before the start of the
scanning operation, and *always* ungets data which matches the input
at that point, so the code always hits the block which backs up over
the input data and never hits the block which uses the _ubuf field.

In addition, the svfwscanf code will always start with the unget
buffer empty, so the ungetwc replacement never needs to support an
unget buffer at all.

Simplify the code by removing support for everything other than
backing up over the input data, leaving the check to make sure it
doesn't get underflowed in case the vfscanf code has a bug in it.

Signed-off-by: Keith Packard <[email protected]>
---
 newlib/libc/stdio/vfwscanf.c | 40 +++---------------------------------
 1 file changed, 3 insertions(+), 37 deletions(-)

diff --git a/newlib/libc/stdio/vfwscanf.c b/newlib/libc/stdio/vfwscanf.c
index f00d41a09..e9e00dfec 100644
--- a/newlib/libc/stdio/vfwscanf.c
+++ b/newlib/libc/stdio/vfwscanf.c
@@ -272,49 +272,15 @@ _sungetwc_r (struct _reent *data,
   /* After ungetc, we won't be at eof anymore */
   fp->_flags &= ~__SEOF;
 
-  /*
-   * If we are in the middle of ungetwc'ing, just continue.
-   * This may require expanding the current ungetc buffer.
+  /* All ungetwc usage in scanf un-gets the current character, so
+   * just back up over the string if we aren't at the start
    */
-
-  if (HASUB (fp))
-    {
-      if (fp->_r >= fp->_ub._size && __submore (data, fp))
-        {
-          return EOF;
-        }
-      fp->_p -= sizeof (wchar_t);
-      *fp->_p = (wchar_t) wc;
-      fp->_r += sizeof (wchar_t);
-      return wc;
-    }
-
-  /*
-   * If we can handle this by simply backing up, do so,
-   * but never replace the original character.
-   * (This makes swscanf() work when scanning `const' data.)
-   */
-
-  if (fp->_bf._base != NULL && fp->_p > fp->_bf._base
-      && ((wchar_t *)fp->_p)[-1] == wc)
+  if (fp->_bf._base != NULL && fp->_p > fp->_bf._base)
     {
       fp->_p -= sizeof (wchar_t);
       fp->_r += sizeof (wchar_t);
-      return wc;
     }
 
-  /*
-   * Create an ungetc buffer.
-   * Initially, we will use the `reserve' buffer.
-   */
-
-  fp->_ur = fp->_r;
-  fp->_up = fp->_p;
-  fp->_ub._base = fp->_ubuf;
-  fp->_ub._size = sizeof (fp->_ubuf);
-  fp->_p = &fp->_ubuf[sizeof (fp->_ubuf) - sizeof (wchar_t)];
-  *(wchar_t *) fp->_p = wc;
-  fp->_r = 2;
   return wc;
 }
 
-- 
2.32.0

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEEw4O3eCVWE9/bQJ2R2yIaaQAAABEFAmEcCc0ACgkQ2yIaaQAA
ABEBKRAAh4KPGL934f+sD2+auzAG8iERGoTNiNjvOh0eY3v4fFEV41MByOuUMWIG
Lz9Xksiuun5dpM/2aPlQPHwnPVipYBvjqEUwA6PGJMKezF2/2rnX4HMEBVuoMF37
otE+rwJRnKHudSxBSzGIiQlj7mihvErlJ2GqTKujp/csWqyPdFfSRrtFGwAp7Xfs
hi6T+ubH46cJvJOVDqCkoPb/ldA3ahUHnbx5+D2bNe8XdywQxYYaaYBNL01vSkAu
yVm7JIu8NRS6aUKUQfNkQ8mQA+/G2M3cOybFCRwvQcj8gJwAm3xJnvwBSGOV0J2h
eP3c/UrXXqn9hLGAfRh3FntWWsJ8wG3JQOccIUS4+r7KzOxy2TkmUVAYeRFdI9yB
rTYMQUKOKb0P5LBi6dlYc194Nc1IXgJcmR34JkPifN/5vRw5GgJ68A9TSIFylLRt
rMRcrhAPNtRxY/uQLCvx3xO+G6jsekB7tLMoo7nksyzoC66aKqY5go9N4ILp4Vt3
SO0ed1ZR3/0Wt9H10VX6i+8T5i0ma/tLLSxkXEInRHbz2XHoEHsIYXJ+kx1rxBX7
pa8g/Zrk0uGJcoDbo/E6Vmh6gjId9BGoPQctAwQemF6C+pkzIwxtATUuQWT+w1fe
/kum6TxQON/rp7pQ8h306t0/FIXx7ZXapiH3gtA0TNZ1McUbHEE=
=Eg/A
-----END PGP SIGNATURE-----