[FFmpeg-cvslog] fraps: check for overread

[Reimar Döffinger]

On Wed, Nov 09, 2011 at 08:05:04PM +0100, Michael Niedermayer wrote:
> 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

Ok, fine. I still intend to bring this up next time I suggest
introducing proper flagging of decode issues instead of only "fail" via
return value and everyone's again more interesting in fixing their
special case only...