[FFmpeg-devel] [libav-devel] [PATCH 5/6] Fixed segfaults on corruped smaker streams in the decoder.

[Michael Niedermayer]

On Mon, Sep 12, 2011 at 11:43:18PM +0200, Laurent Aimar wrote:
> On Mon, Sep 12, 2011 at 11:28:44PM +0200, Reimar Döffinger wrote:
> > On Sun, Sep 11, 2011 at 07:56:46PM +0200, Laurent Aimar wrote:
> > > @@ -653,6 +659,8 @@ static int smka_decode_frame(AVCodecContext *avctx, void *data, int *data_size,
> > >      } else { //8-bit data
> > >          for(i = stereo; i >= 0; i--)
> > >              pred[i] = get_bits(&gb, 8);
> > > +        if (stereo + unp_size > data_size)
> > > +            return -1;
> > 
> > This can overflow.
> > if (unp_size < 0 || unp_size > data_size - stereo)
> > should probably be safe.
>  No it doesn't because of the surrounding code BUT I saw a bug
> in this patch (data_size is a pointer, a '*' is missing)

fixed


> 
>  It can also be made a bit simpler. I will propose a better patch
> later.

i guess you plan something like: ?

--- a/libavcodec/smacker.c
+++ b/libavcodec/smacker.c
@@ -594,7 +594,7 @@ static int smka_decode_frame(AVCodecContext *avctx, void *data, int *data_size,
     }
     stereo = get_bits1(&gb);
     bits = get_bits1(&gb);
-    if (unp_size & 0xC0000000 || unp_size > *data_size) {
+    if (unp_size & 0xC0000000 || unp_size + stereo > *data_size) {
         av_log(avctx, AV_LOG_ERROR, "Frame is too large to fit in buffer\n");
         return -1;
     }
@@ -659,8 +659,6 @@ static int smka_decode_frame(AVCodecContext *avctx, void *data, int *data_size,
     } else { //8-bit data
         for(i = stereo; i >= 0; i--)
             pred[i] = get_bits(&gb, 8);
-        if (stereo + unp_size > *data_size)
-            return -1;
         for(i = 0; i < stereo; i++)
             *samples8++ = pred[i];
         for(i = 0; i < unp_size; i++) {

i can just commit it already as ive already written it :)

[..]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No great genius has ever existed without some touch of madness. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110912/c68d9f7b/attachment.asc>