[Speex-dev] [PATCH]Add address overflow check

[Ruikai Liu]

I'm not familiar with the code, but it seems that the new address is
accessed for `nb_fields`:

   end = c+length;

   len=readint(c, 0);

   c+=4;

   if (len < 0 || c+len>end)

   {

      fprintf (stderr, "Invalid/corrupted comments\n");

      return;

   }

   fwrite(c, 1, len, stderr);

   c+=len;

   fprintf (stderr, "\n");

   if (c+4>end)

   {

      fprintf (stderr, "Invalid/corrupted comments\n");

      return;

   }

   nb_fields=readint(c, 0);

So if `c+len` overflows and becomes some really small value, say `NULL`,
then `readint()` would result in segfault.

On Fri, Feb 9, 2018 at 11:56 PM, Jean-Marc Valin <jmvalin at jmvalin.ca> wrote:

> Pointers are unsigned so this shouldn't be an issue. I suspect you're
> being hit by something else. That or your compiler is really broken.
>
> Cheers,
>
>         Jean-Marc
>
> On 02/09/2018 04:42 AM, Ruikai Liu wrote:
> > Hi,
> >
> > I came into a crash when using 32-bit `speexdec` and found that there's
> > an address overflow in function `print_comments()`:
> >
> > staticvoidprint_comments(char*comments, intlength)
> >
> > {
> >
> >    char*c=comments;
> >
> >    intlen, i, nb_fields;
> >
> >    char*end;
> >
> >
> >    if(length<8)
> >
> >    {
> >
> >       fprintf (stderr, "Invalid/corrupted comments\n");
> >
> >       return;
> >
> >    }
> >
> >    end = c+length;
> >
> >    len=readint(c, 0);
> >
> >    c+=4;
> >
> > // 'c+len' MAY OVERFLOW
> >
> >    if(len < 0|| c+len>end)
> >
> >    {
> >
> >       fprintf (stderr, "Invalid/corrupted comments\n");
> >
> >       return;
> >
> >    }
> >
> >
> > The pointer `c` happened to be greater than `0x80000000` and the sum
> > overflowed, even though `length` is positive.
> >
> > Here's the patch code:
> >
> > *diff --git a/src/speexdec.c b/src/speexdec.c*
> >
> > *index 4721dc1..18786f1 100644*
> >
> > *--- a/src/speexdec.c*
> >
> > *+++ b/src/speexdec.c*
> >
> > @@ -105,7 +105,7 @@static void print_comments(char *comments, int length)
> >
> >     end = c+length;
> >
> >     len=readint(c, 0);
> >
> >     c+=4;
> >
> > -   if (len < 0 || c+len>end)
> >
> > +   if (len < 0 || c+len>end || c+len<c)
> >
> >     {
> >
> >        fprintf (stderr, "Invalid/corrupted comments\n");
> >
> >        return;
> >
> > @@ -129,7 +129,7 @@static void print_comments(char *comments, int length)
> >
> >        }
> >
> >        len=readint(c, 0);
> >
> >        c+=4;
> >
> > -      if (len < 0 || c+len>end)
> >
> > +      if (len < 0 || c+len>end || c+len<c)
> >
> >        {
> >
> >           fprintf (stderr, "Invalid/corrupted comments\n");
> >
> >           return;
> >
> >
> > Thanks!
> >
> > --
> > Best regards,
> >
> > Ruikai Liu
> >
> >
> > _______________________________________________
> > Speex-dev mailing list
> > Speex-dev at xiph.org
> > http://lists.xiph.org/mailman/listinfo/speex-dev
> >
>



-- 
Best regards,

Ruikai Liu
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.xiph.org/pipermail/speex-dev/attachments/20180211/d112226a/attachment.html>