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

Michael Niedermayer michaelni
Thu Jan 6 23:58:35 CET 2011

On Thu, Jan 06, 2011 at 05:54:52PM -0500, Daniel Kang wrote:

On Thu, Jan 6, 2011 at 5:49 PM, Michael Niedermayer wrote:

> > case 0: //Normal Chain > > + if (buf + length >= bufend) { > > 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 bufend > > This is unlikely in practice but its easy to avoid this problem by using > > if(length >= bufend - 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 bfibuffersanitycheck.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>

More information about the ffmpeg-devel mailing list