Patchwork [libcpp] : Avoid crash in interpret_float_suffix

login
register
mail settings
Submitter Tristan Gingold
Date May 4, 2012, 12:23 p.m.
Message ID <1BC426F9-437D-41CB-9581-47CA0E222A83@adacore.com>
Download mbox | patch
Permalink /patch/156894/
State New
Headers show

Comments

Tristan Gingold - May 4, 2012, 12:23 p.m.
Hi,

the function libcpp/expr.c:interpret_float_suffix allows its argument LEN to be 0, but in this case it tries to read before the buffer S.  It is not a real issue, except in case of overflow:  on VMS with 64bit pointers but 32bit size_t, the following code:
  s[len-1]
is evaluated as
  s[0xffffffff]
which is likely (and does) crash cc1.

To avoid this nasty effect, I just added a guard.

Bootstrapped and regtested on i386/GNU linux.

Ok for trunk ?

Tristan.

libcpp/
2012-05-04  Tristan Gingold  <gingold@adacore.com>

	* expr.c (interpret_float_suffix): Add a guard.
Dodji Seketeli - May 4, 2012, 2:03 p.m.
Tristan Gingold <gingold@adacore.com> a écrit:

> the function libcpp/expr.c:interpret_float_suffix allows its argument
> LEN to be 0, but in this case it tries to read before the buffer S.
> It is not a real issue, except in case of overflow: on VMS with 64bit
> pointers but 32bit size_t, the following code: s[len-1] is evaluated
> as s[0xffffffff] which is likely (and does) crash cc1.
>
> To avoid this nasty effect, I just added a guard.
>
> Bootstrapped and regtested on i386/GNU linux.
>
> Ok for trunk ?

I can not approve or deny this patch, but for what it's worth, it looks
fine to me.

[...]

> +++ b/libcpp/expr.c
> @@ -110,12 +110,13 @@ interpret_float_suffix (const uchar *s, size_t len)
>      }
>  
>    /* Recognize a fixed-point suffix.  */
> -  switch (s[len-1])
> -    {
> -    case 'k': case 'K': flags = CPP_N_ACCUM; break;
> -    case 'r': case 'R': flags = CPP_N_FRACT; break;
> -    default: break;
> -    }
> +  if (len != 0)
> +    switch (s[len-1])
> +      {
> +      case 'k': case 'K': flags = CPP_N_ACCUM; break;
> +      case 'r': case 'R': flags = CPP_N_FRACT; break;
> +      default: break;
> +      }
>  
>    /* Continue processing a fixed-point suffix.  The suffix is case
>       insensitive except for ll or LL.  Order is significant.  */

Thanks.
Tom Tromey - May 8, 2012, 3:39 p.m.
>>>>> "Tristan" == Tristan Gingold <gingold@adacore.com> writes:

Tristan> 2012-05-04  Tristan Gingold  <gingold@adacore.com>
Tristan> 	* expr.c (interpret_float_suffix): Add a guard.

Ok.

Tom
Tristan Gingold - May 10, 2012, 8:04 a.m.
On May 8, 2012, at 5:39 PM, Tom Tromey wrote:

>>>>>> "Tristan" == Tristan Gingold <gingold@adacore.com> writes:
> 
> Tristan> 2012-05-04  Tristan Gingold  <gingold@adacore.com>
> Tristan> 	* expr.c (interpret_float_suffix): Add a guard.
> 
> Ok.

Thanks, now committed.

Patch

diff --git a/libcpp/expr.c b/libcpp/expr.c
index d56e56a..ca1c3d1 100644
--- a/libcpp/expr.c
+++ b/libcpp/expr.c
@@ -110,12 +110,13 @@  interpret_float_suffix (const uchar *s, size_t len)
     }
 
   /* Recognize a fixed-point suffix.  */
-  switch (s[len-1])
-    {
-    case 'k': case 'K': flags = CPP_N_ACCUM; break;
-    case 'r': case 'R': flags = CPP_N_FRACT; break;
-    default: break;
-    }
+  if (len != 0)
+    switch (s[len-1])
+      {
+      case 'k': case 'K': flags = CPP_N_ACCUM; break;
+      case 'r': case 'R': flags = CPP_N_FRACT; break;
+      default: break;
+      }
 
   /* Continue processing a fixed-point suffix.  The suffix is case
      insensitive except for ll or LL.  Order is significant.  */