opus - Nov 2012 - Integer overflow in opus_packet_parse_impl

Hello,

there is a chance for an integer overflow in opus_packet_parse_impl():

   int padding=0;
   int p;
   do {
      if (len<=0)
         return OPUS_INVALID_PACKET;
      p = *data++;
      len--;
      padding += p==255 ? 254: p;
   } while (p==255);
   len -= padding;
   ...
   if (len<0)
      return OPUS_INVALID_PACKET;

When ~16 MB of 0xff bytes is fed to the decoder, the padding variable overflows
which leads to out-of-bounds read. It would be good to add a check for the
overflow even if most cases won't allow that much data to be fed to the
decoder.


Here is a reproducer:

#include <string.h>
#include "opus.h"

unsigned char in_buf[16909318];
unsigned short out_buf[11520];

int main() {
  OpusDecoder *decoder;
  int result;
  int error;

  in_buf[0] = 0xff;
  in_buf[1] = 0x41;
  memset(in_buf + 2, 0xff, 16909315);
  in_buf[16909317] = 0x0b;

  decoder = opus_decoder_create(48000, 2, &error);
  result = opus_decode(decoder, in_buf, 16909318, out_buf, 5760, 0);
}


Here is the patch I'm suggesting:

--- opus_decoder_old.c	2012-11-12 23:35:03.289595241 -0800
+++ opus_decoder.c	2012-11-12 23:36:44.550437586 -0800
@@ -34,6 +34,7 @@
 #endif

 #include <stdarg.h>
+#include <limits.h>
 #include "celt.h"
 #include "opus.h"
 #include "entdec.h"
@@ -619,6 +620,8 @@
                return OPUS_INVALID_PACKET;
             p = *data++;
             len--;
+            if (padding > INT_MAX - p)
+               return OPUS_INVALID_PACKET;
             padding += p==255 ? 254: p;
          } while (p==255);
          len -= padding;

Cheers,
J?ri

On 12-11-13 12:01 AM, J?ri Aedla wrote:
> Hello,
> 
> there is a chance for an integer overflow in opus_packet_parse_impl():
Thanks for the report! Sorry it got stuck in the mod queue for so long;
we only check it periodically.

We've committed a simpler fix as
http://git.xiph.org/?p=opus.git;a=commitdiff;h=9345aaa5ca1c2fb7d62981b2a538e0ce20612c38

 -r