diff mbox

Fix for heap overflow in wscanf (BZ 16618)

Message ID 54CFCEB1.8090301@cs.ucla.edu
State Superseded
Headers

Commit Message

Paul Eggert Feb. 2, 2015, 7:23 p.m. UTC 
  On 02/02/2015 08:57 AM, Paul Pluzhnikov wrote:
> +	  if (__glibc_unlikely (wpmax > SIZE_MAX / 2)			    \
> +	      || __glibc_unlikely (newsize > SIZE_MAX / sizeof (CHAR_T)))   \

"if (__glibc_unlikely (wpmax > SIZE_MAX / sizeof (CHAR_T) / 2)" is 
shorter and faster.  Also, the proposed patch duplicates the use_malloc 
/ free / done code, which is messy.  And there's an existing use of 
'use_malloc' which makes sense only if the code ignores overflow, which 
it no longer does.

So, how about the attached (untested) patch to vfscanf.c instead? It's 
simpler.  It does rely on realloc (wp, SIZE_MAX) returning NULL, but 
that's safe in glibc.
diff mbox

Patch

diff --git a/stdio-common/vfscanf.c b/stdio-common/vfscanf.c
index cd129a8..0e204e7 100644
--- a/stdio-common/vfscanf.c
+++ b/stdio-common/vfscanf.c
@@ -272,9 +272,10 @@  _IO_vfscanf_internal (_IO_FILE *s, const char *format, _IO_va_list argptr,
       if (__glibc_unlikely (wpsize == wpmax))				      \
 	{								    \
 	  CHAR_T *old = wp;						    \
-	  size_t newsize = (UCHAR_MAX + 1 > 2 * wpmax			    \
-			    ? UCHAR_MAX + 1 : 2 * wpmax);		    \
-	  if (use_malloc || !__libc_use_alloca (newsize))		    \
+	  bool fits = __glibc_likely (wpmax <= SIZE_MAX / sizeof (CHAR_T) / 2); \
+	  size_t wpneed = MAX (UCHAR_MAX + 1, 2 * wpmax);		    \
+	  size_t newsize = fits ? wpneed * sizeof (CHAR_T) : SIZE_MAX;	    \
+	  if (!__libc_use_alloca (newsize))				    \
 	    {								    \
 	      wp = realloc (use_malloc ? wp : NULL, newsize);		    \
 	      if (wp == NULL)						    \
@@ -286,14 +287,13 @@  _IO_vfscanf_internal (_IO_FILE *s, const char *format, _IO_va_list argptr,
 		}							    \
 	      if (! use_malloc)						    \
 		MEMCPY (wp, old, wpsize);				    \
-	      wpmax = newsize;						    \
+	      wpmax = wpneed;						    \
 	      use_malloc = true;					    \
 	    }								    \
 	  else								    \
 	    {								    \
 	      size_t s = wpmax * sizeof (CHAR_T);			    \
-	      wp = (CHAR_T *) extend_alloca (wp, s,			    \
-					     newsize * sizeof (CHAR_T));    \
+	      wp = (CHAR_T *) extend_alloca (wp, s, newsize);		    \
 	      wpmax = s / sizeof (CHAR_T);				    \
 	      if (old != NULL)						    \
 		MEMCPY (wp, old, wpsize);				    \