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

[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

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

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- 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/20110603/bbecb897/attachment.asc>