[FFmpeg-devel] [PATCH] Fix a couple of errors with bad Vorbis headers (original) (raw)

Michael Niedermayer michaelni
Sat Jan 15 15:56:50 CET 2011

On Mon, Jan 10, 2011 at 06:13:21PM -0800, Frank Barchard wrote:

This update removes a shift, and addresses Reimar's safety concern. On corrupt files, vorbisdecodeinit now returns with -1;

vorbisdec.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) 4a0bdc07bb1a5259fc8b42809666f2aa21a5d9df 21vorbisoverflow.2.patch diff -wurp -N orig/libavcodec/vorbisdec.c ffmpeg-mt/libavcodec/vorbisdec.c --- orig/libavcodec/vorbisdec.c 2011-01-10 17:27:28 -0800 +++ ffmpeg-mt/libavcodec/vorbisdec.c 2011-01-10 17:28:18 -0800 @@ -483,6 +483,7 @@ static int vorbisparsesetuphdrfloors if (floorsetup->floortype == 1) { int maximumclass = -1; uintfast8t rangebits; + uintfast32t rangemax; uintfast16t floor1values = 2;

floorsetup->decode = vorbisfloor1decode; @@ -534,8 +535,15 @@ static int vorbisparsesetuphdrfloors

rangebits = getbits(gb, 4); + rangemax = (1 << rangebits);_ _+ if (rangemax > vc->blocksize[1] / 2) { + avlog(vc->avccontext, AVLOGERROR, + "Floor value is too large for blocksize: %d (%d)\n", + rangemax, vc->blocksize[1] / 2); + return -1; + } floorsetup->data.t1.list[0].x = 0; - floorsetup->data.t1.list[1].x = (1 << rangebits);_ _+ floorsetup->data.t1.list[1].x = rangemax; for (j = 0; j < floorsetup->data.t1.partitions; ++j) { for (k = 0; k < floorsetup->data.t1.classdimensions[floorsetup->data.t1.partitionclass[j]]; ++k, ++floor1values) {

these 2 hunks look ok to me though ive not deeply investigated. They definitly should be applied ASAP though as this is a security fix Further investigation can be done once this is in

> @@ -653,7 +661,7 @@ static int vorbis_parse_setup_hdr_residu

ressetup->partitionsize = getbits(gb, 24) + 1; /* Validations to prevent a buffer overflow later. */ if (ressetup->begin>ressetup->end || - ressetup->end > vc->avccontext->channels * vc->blocksize[1] / (ressetup->type == 2 ? 1 : 2) || + ressetup->end > vc->avccontext->channels * vc->blocksize[1] / 2 || (ressetup->end-ressetup->begin) / ressetup->partitionsize > VMAXPARTITIONS) { avlog(vc->avccontext, AVLOGERROR, "partition out of bounds: type, begin, end, size, blocksize: %"PRIdFAST16", %"PRIdFAST32", %"PRIdFAST32", %u, %"PRIdFAST32"\n", ressetup->type, ressetup->begin, ressetup->end, ressetup->partitionsize, vc->blocksize[1] / 2); return -1;

this is a mystery to me what does this fix?

What i found when looking at the code is that ptns_to_read is uint_fast16_t but values stored in there are tested against #define V_MAX_PARTITIONS (1 << 20) thats definitly not ok

[...]

Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

There seems to be only one solution to NIH syndrom, ... a shooting squad -------------- 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/20110115/0e52ff09/attachment.pgp>

More information about the ffmpeg-devel mailing list