[FFmpeg-devel] [PATCH] add sanity checks to truemotion2 (issue 2512) (original) (raw)

Daniel Kang daniel.d.kang
Sun Jan 9 01:34:41 CET 2011

On Sat, Jan 8, 2011 at 7:23 PM, Stefano Sabatini < stefano.sabatini-lala at poste.it> wrote:

On date Saturday 2011-01-08 19:10:06 -0500, Daniel Kang encoded: > truemotion2 videos with invalid metadata crashes ffmpeg. There are > multiple places where the decoder (demuxer?) overreads a buffer. The > patch adds several checks for this. Are there any comments?

> From fb6f2b4591c059887fa32c5277aade5964b6bc70 Mon Sep 17 00:00:00 2001 > From: Daniel Kang <daniel.d.kang at gmail.com> > Date: Sat, 8 Jan 2011 16:11:31 -0500 > Subject: [PATCH] Fix several errors in truemotion2 decoding > > --- > libavcodec/truemotion2.c | 16 +++++++++++++--- > 1 files changed, 13 insertions(+), 3 deletions(-) > > diff --git a/libavcodec/truemotion2.c b/libavcodec/truemotion2.c > index 5013a9e..9c08d61 100644 > --- a/libavcodec/truemotion2.c > +++ b/libavcodec/truemotion2.c > @@ -260,7 +260,7 @@ static int tm2readdeltas(TM2Context *ctx, int streamid) { > return 0; > } > > -static int tm2readstream(TM2Context *ctx, const uint8t *buf, int streamid) { > +static int tm2readstream(TM2Context *ctx, const uint8t *buf, int streamid, int bufsize) { Nit++: "{" on following line per K&R style > int i; > int cur = 0; > int skip = 0; > @@ -274,6 +274,11 @@ static int tm2readstream(TM2Context *ctx, const uint8t *buf, int streamid) { > if(len == 0) > return 4; > > + if(len >= INTMAX/4-1 || len < 0 || len > bufsize) { nit: if( > + avlog(ctx->avctx, AVLOGERROR, "Error, invalid stream size.\n"); > + return -1; Please return meaningful error codes, AVERRORINVALIDDATA in this case. > + } > + > toks = AVRB32(buf); buf += 4; cur += 4; > if(toks & 1) { > len = AVRB32(buf); buf += 4; cur += 4; > @@ -313,8 +318,13 @@ static int tm2readstream(TM2Context *ctx, const uint8t *buf, int streamid) { > len = AVRB32(buf); buf += 4; cur += 4; > if(len > 0) { > initgetbits(&ctx->gb, buf, (skip - cur) * 8); > - for(i = 0; i < toks; i++)_ _> + for(i = 0; i < toks; i++) {_ _> + if (getbitsleft(&ctx->gb) <= 0) {_ _> + avlog(ctx->avctx, AVLOGERROR, "Incorrect number of tokens: %i\n", toks); > + return -1; ditto

tm2_read_stream is called from decode_frame, which will error out in the case of -1. If -1 is not returned, decode_frame will continue to decode. For values that are not -1, t is added to skip, which could (potentially) decrementing it, which I do not believe is the correct behavior.

I have updated the patch with the other comments. -------------- next part -------------- A non-text attachment was scrubbed... Name: truemotion2_checks.diff Type: application/octet-stream Size: 2307 bytes Desc: not available URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110108/8e44f541/attachment.obj>

More information about the ffmpeg-devel mailing list