[FFmpeg-cvslog] fraps: check for overread

[Michael Niedermayer]

On Wed, Nov 09, 2011 at 07:18:48PM +0100, Reimar Döffinger wrote:
> On Wed, Nov 09, 2011 at 12:18:45AM +0100, Michael Niedermayer wrote:
> > ffmpeg | branch: master | Michael Niedermayer <michaelni at gmx.at> | Tue Nov  8 22:42:50 2011 +0100| [3bdfef31ac135add243f9ddde99d6b3cee953833] | committer: Michael Niedermayer
> > 
> > fraps: check for overread
> > Fixeds Ticket619
> > 
> > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> > 
> > > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=3bdfef31ac135add243f9ddde99d6b3cee953833
> > ---
> > 
> >  libavcodec/fraps.c      |    2 ++
> >  tests/ref/fate/fraps-v5 |    1 -
> >  2 files changed, 2 insertions(+), 1 deletions(-)
> > 
> > diff --git a/libavcodec/fraps.c b/libavcodec/fraps.c
> > index d6f9a55..0844fa7 100644
> > --- a/libavcodec/fraps.c
> > +++ b/libavcodec/fraps.c
> > @@ -114,6 +114,8 @@ static int fraps2_decode_plane(FrapsContext *s, uint8_t *dst, int stride, int w,
> >              else if(Uoff) dst[i] += 0x80;
> >          }
> >          dst += stride;
> > +        if(get_bits_left(&gb) < 0)
> > +            return -1;
> >      }
> >      free_vlc(&vlc);
> >      return 0;
> 
> This is wrong.
> First, checking only once per line is questionable at least, and I'd
> expect it can overread and crash with certain VLC tables.
> More importantly you skip the free_vlc and it will thus leak memory.

fixed locally


> Philosophically I also disagree a bit with failing completely when
> we (probably) managed to decode a large part of the frame perfectly
> fine, doubly so because fraps reuses the previous frame which means
> we actually would have kind of error concealment.

simply ignoring the failure end exiting causes 3/4 of the frame to be
trashed from the sample of ticket619
i see some in place yuv-rgb transform, which may be the cause but i
did not investigate

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

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.
-------------- 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-cvslog/attachments/20111109/2f1173d2/attachment.asc>