[FFmpeg-devel] [PATCH] fix for bfi buffer overread (roundup issue 2497)

[Michael Niedermayer]

On Thu, Jan 06, 2011 at 05:54:52PM -0500, Daniel Kang wrote:
> On Thu, Jan 6, 2011 at 5:49 PM, Michael Niedermayer <michaelni at gmx.at>wrote:
> 
> >  >          case 0:                //Normal Chain
> > > +            if (buf + length >= buf_end) {
> >
> > This has a very small chance that buf + length overflows, that is if buf is
> > close to 0xFFFFFFFF on a 32bit system or 0xFFFFFFFFFFFFFFFF on 64bit then
> > buf + length can be smaller than buf and thus also smaller than buf_end
> >
> > This is unlikely in practice but its easy to avoid this problem by using
> >
> > if(length >= buf_end - buf)
> >
> > as a rule of thumb, the variable to be checked for validity should be
> > alone on one side of the </>/<=/>= to avoid this kind of problem
> >
> > The same issue exists with code like
> > if(something * sizeof(some struct) > foo)
> > vs.
> > if(something > foo / sizeof(some struct))
> 
> 
>  Thank you for the tip, I had not thought of that.
> 
> I have updated the patch.

>  bfi.c |   11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> efe1cb448387bbfe7e040bed41623cddd981ab78  bfi_buffer_sanity_check.diff
> From 3981253d187c197429feaf6fe051799c261a29e1 Mon Sep 17 00:00:00 2001
> From: Daniel Kang <daniel.d.kang at gmail.com>
> Date: Wed, 5 Jan 2011 23:46:33 -0500
> Subject: [PATCH] Sanity check on buffer reads

lgtm

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

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110106/50fac3db/attachment.pgp>