[FFmpeg-devel] [PATCH] indeo3: add out-of-buffer write check

[Michael Niedermayer]

On Sat, Jun 04, 2011 at 06:16:22PM +0200, Maxim wrote:
> Am 03.06.2011 19:48, schrieb Michael Niedermayer:
> > On Tue, May 24, 2011 at 11:46:40AM +0200, Maxim wrote:
> >   
> >> Am 23.05.2011 21:03, schrieb Michael Niedermayer:
> >>     
> >>> On Mon, May 23, 2011 at 05:40:11PM +0200, Laurent Aimar wrote:
> >>>   
> >>>       
> >>>> Hi,
> >>>>
> >>>>     
> >>>>         
> >>>>> Prevent out-of-buffer writes. In particular fix smclocki32.avi.1.1
> >>>>> crash, fix trac issue #114, roundup issue #1482.
> >>>>> ---
> >>>>>  libavcodec/indeo3.c |    3 +++
> >>>>>  1 files changed, 3 insertions(+), 0 deletions(-)
> >>>>>
> >>>>> diff --git a/libavcodec/indeo3.c b/libavcodec/indeo3.c
> >>>>> index b74fcf7..8e55fbe 100644
> >>>>> --- a/libavcodec/indeo3.c
> >>>>> +++ b/libavcodec/indeo3.c
> >>>>> @@ -213,6 +213,7 @@ static void iv_Decode_Chunk(Indeo3DecodeContext *s,
> >>>>>      int *width_tbl, width_tbl_arr[10];
> >>>>>      const signed char *ref_vectors;
> >>>>>      uint8_t *cur_frm_pos, *ref_frm_pos, *cp, *cp2;
> >>>>> +    uint8_t *cur_end = cur + width*height + width;
> >>>>>      uint32_t *cur_lp, *ref_lp;
> >>>>>      const uint32_t *correction_lp[2], *correctionloworder_lp[2], *correctionhighorder_lp[2];
> >>>>>      uint8_t *correction_type_sp[2];
> >>>>> @@ -359,6 +360,8 @@ static void iv_Decode_Chunk(Indeo3DecodeContext *s,
> >>>>>                              k = *buf1++;
> >>>>>                              cur_lp = ((uint32_t *)cur_frm_pos) + width_tbl[lp2];
> >>>>>                              ref_lp = ((uint32_t *)ref_frm_pos) + width_tbl[lp2];
> >>>>> +                            if ((uint8_t *)cur_lp >= cur_end-3)
> >>>>> +                                break;
> >>>>>       
> >>>>>           
> >>>> If the overflow can happen, then IMO this check is not robust enough, you can
> >>>> have an integer overflow when computing cur_lp address.
> >>>>  Also while at it, it seems that width_tbl can be < 0, and I am not sure it is
> >>>> correctly checked.
> >>>>     
> >>>>         
> >>> Iam sure there are alot more bugs in there
> >>> maxim has a cleaner decoder and iam hoping he will post it soon so we
> >>> can decide which to keep before putting too much time in one that we
> >>> might remove later
> >>>   
> >>>       
> >> I'm sorry for not posting the code earlier but I have to do some checks
> >> (mainly on my "big-endian" machine) and final cleanups. Plz give me a
> >> week for that. You might use that time for doing another important work...
> >>     
> > ping
> >   
> 
> Unfortunately my code causes the FFmpeg executable to crash after
> decoding with the new git snapshot. That means there are bugs somewhere.
> I cannot submit the code until that's fixed. So please be patient and
> allow me abit more time...
> I'll start a new thread then, so that one can be closed...

ok, no problem

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

The bravest are surely those who have the clearest vision
of what is before them, glory and danger alike, and yet
notwithstanding go out to meet it. -- Thucydides
-------------- 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/20110604/9317a41d/attachment.asc>