On Tue, Feb 03, 2015 at 07:04:11PM +0100, wm4 wrote: > This could overflow and crash at least on 32 bit systems. > --- > libavformat/mpc8.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavformat/mpc8.c b/libavformat/mpc8.c > index a15dc25..d6ca338 100644 > --- a/libavformat/mpc8.c > +++ b/libavformat/mpc8.c > @@ -91,7 +91,7 @@ static int mpc8_probe(AVProbeData *p) > size = bs_get_v(&bs); > if (size < 2) > return 0; > - if (bs + size - 2 >= bs_end) > + if (size >= bs_end - bs + 2) > return AVPROBE_SCORE_EXTENSION - 1; // seems to be valid MPC but > no header yet
Seems ok to me, but for consistency/ease of checking I'd suggest changing while (bs < bs_end + 3) to this style as well. However there is one more issue: bs_get_v can overflow. And as the compiler can assume that signed overflow will not happen, that case is not certain to be caught by the size < 2 check, and thus these cases might escape all checks. Stupid question: Why do we support size values of whole 64 bits? Nobody has invented any storage media where one could store that much. Changing the limit in bs_get_v from 10 to e.g. 6 or 7 should avoid the issue... _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel