diff mbox

Fix for heap overflow in wscanf (BZ 16618)

Message ID CALoOobPgvuBLTk4GzOchr792MHNi1yLgsO5Jqf8MPvY+bk544Q@mail.gmail.com
State New
Headers show

Commit Message

Paul Pluzhnikov Feb. 1, 2015, 8:52 p.m. UTC 
Greetings,

Attached patch is a rather obvious fix for BZ 16618.
I believe this bug deserves a CVE (I've asked for one), and the fix
should definitely go into 2.21.

Tested on Linux/x86_64, no new failures.

Thanks,
--


Paul Pluzhnikov


2015-02-01  Paul Pluzhnikov  <ppluzhnikov@google.com>

	[BZ #16618]
	* stdio-common/vfscanf.c (ADDW): Correct alloca size check and
	fix heap buffer overflow.

Comments

H.J. Lu Feb. 1, 2015, 9:36 p.m. UTC | #1 
On Sun, Feb 1, 2015 at 12:52 PM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
> Greetings,
>
> Attached patch is a rather obvious fix for BZ 16618.
> I believe this bug deserves a CVE (I've asked for one), and the fix
> should definitely go into 2.21.
>
> Tested on Linux/x86_64, no new failures.
>
> Thanks,
> --
>
>
> Paul Pluzhnikov
>
>
> 2015-02-01  Paul Pluzhnikov  <ppluzhnikov@google.com>
>
>         [BZ #16618]
>         * stdio-common/vfscanf.c (ADDW): Correct alloca size check and
>         fix heap buffer overflow.

Please include the testcase in BZ #16618.
Rich Felker Feb. 2, 2015, 5:09 a.m. UTC | #2 
On Sun, Feb 01, 2015 at 12:52:48PM -0800, Paul Pluzhnikov wrote:
> Greetings,
> 
> Attached patch is a rather obvious fix for BZ 16618.
> I believe this bug deserves a CVE (I've asked for one), and the fix
> should definitely go into 2.21.
> 
> Tested on Linux/x86_64, no new failures.
>  * A new semaphore algorithm has been implemented in generic C code for all
>    machines. Previous custom assembly implementations of semaphore were
> diff --git a/stdio-common/vfscanf.c b/stdio-common/vfscanf.c
> index cd129a8..b2ad0c5 100644
> --- a/stdio-common/vfscanf.c
> +++ b/stdio-common/vfscanf.c
> @@ -274,9 +274,9 @@ _IO_vfscanf_internal (_IO_FILE *s, const char *format, _IO_va_list argptr,
>  	  CHAR_T *old = wp;						    \
>  	  size_t newsize = (UCHAR_MAX + 1 > 2 * wpmax			    \
>  			    ? UCHAR_MAX + 1 : 2 * wpmax);		    \
> -	  if (use_malloc || !__libc_use_alloca (newsize))		    \
> +	  if (use_malloc || !__libc_use_alloca (newsize * sizeof (CHAR_T))) \
>  	    {								    \
> -	      wp = realloc (use_malloc ? wp : NULL, newsize);		    \
> +	      wp = realloc (use_malloc ? wp : NULL, newsize * sizeof (CHAR_T));	\

Offhand, the multiplication newsize * sizeof (CHAR_T) looks like a
potential integer overflow. Are you sure it's okay?

Rich
diff mbox

Patch

diff --git a/NEWS b/NEWS
index c91b9fc..85b2948 100644
--- a/NEWS
+++ b/NEWS
@@ -10,15 +10,16 @@  Version 2.21
 * The following bugs are resolved with this release:
 
   6652, 10672, 12674, 12847, 12926, 13862, 14132, 14138, 14171, 14498,
-  15215, 15378, 15884, 16009, 16418, 16191, 16469, 16576, 16617, 16619,
-  16657, 16740, 16857, 17192, 17266, 17273, 17344, 17363, 17370, 17371,
-  17411, 17460, 17475, 17485, 17501, 17506, 17508, 17522, 17555, 17570,
-  17571, 17572, 17573, 17574, 17582, 17583, 17584, 17585, 17589, 17594,
-  17601, 17608, 17616, 17625, 17630, 17633, 17634, 17635, 17647, 17653,
-  17657, 17658, 17664, 17665, 17668, 17682, 17702, 17717, 17719, 17722,
-  17723, 17724, 17725, 17732, 17733, 17744, 17745, 17746, 17747, 17748,
-  17775, 17777, 17780, 17781, 17782, 17791, 17793, 17796, 17797, 17801,
-  17803, 17806, 17834, 17844, 17848, 17868, 17869, 17870, 17885, 17892.
+  15215, 15378, 15884, 16009, 16418, 16191, 16469, 16576, 16617, 16618,
+  16619, 16657, 16740, 16857, 17192, 17266, 17273, 17344, 17363, 17370,
+  17371, 17411, 17460, 17475, 17485, 17501, 17506, 17508, 17522, 17555,
+  17570, 17571, 17572, 17573, 17574, 17582, 17583, 17584, 17585, 17589,
+  17594, 17601, 17608, 17616, 17625, 17630, 17633, 17634, 17635, 17647,
+  17653, 17657, 17658, 17664, 17665, 17668, 17682, 17702, 17717, 17719,
+  17722, 17723, 17724, 17725, 17732, 17733, 17744, 17745, 17746, 17747,
+  17748, 17775, 17777, 17780, 17781, 17782, 17791, 17793, 17796, 17797,
+  17801, 17803, 17806, 17834, 17844, 17848, 17868, 17869, 17870, 17885,
+  17892.
 
 * A new semaphore algorithm has been implemented in generic C code for all
   machines. Previous custom assembly implementations of semaphore were
diff --git a/stdio-common/vfscanf.c b/stdio-common/vfscanf.c
index cd129a8..b2ad0c5 100644
--- a/stdio-common/vfscanf.c
+++ b/stdio-common/vfscanf.c
@@ -274,9 +274,9 @@  _IO_vfscanf_internal (_IO_FILE *s, const char *format, _IO_va_list argptr,
 	  CHAR_T *old = wp;						    \
 	  size_t newsize = (UCHAR_MAX + 1 > 2 * wpmax			    \
 			    ? UCHAR_MAX + 1 : 2 * wpmax);		    \
-	  if (use_malloc || !__libc_use_alloca (newsize))		    \
+	  if (use_malloc || !__libc_use_alloca (newsize * sizeof (CHAR_T))) \
 	    {								    \
-	      wp = realloc (use_malloc ? wp : NULL, newsize);		    \
+	      wp = realloc (use_malloc ? wp : NULL, newsize * sizeof (CHAR_T));	\
 	      if (wp == NULL)						    \
 		{							    \
 		  if (use_malloc)					    \