[FFmpeg-devel] [PATCH] FLAC decoder segfault reading metadata
Michael Niedermayer
michaelni
Tue Oct 2 23:54:59 CEST 2007
On Sun, Sep 30, 2007 at 11:11:12PM -0400, Justin Ruggles wrote:
> Well...I thought I had looked this over long enough, but I guess not. Here
> is a better patch.
>
> -Justin
>
> Index: libavcodec/flac.c
> ===================================================================
> --- libavcodec/flac.c (revision 10629)
> +++ libavcodec/flac.c (working copy)
> @@ -176,7 +176,7 @@
> */
> static int metadata_parse(FLACContext *s)
> {
> - int i, metadata_last, metadata_type, metadata_size, streaminfo_updated=0;
> + int metadata_last, metadata_type, metadata_size, streaminfo_updated=0;
>
> if (show_bits_long(&s->gb, 32) == MKBETAG('f','L','a','C')) {
> skip_bits(&s->gb, 32);
> @@ -191,21 +191,45 @@
> " metadata block: flag = %d, type = %d, size = %d\n",
> metadata_last, metadata_type, metadata_size);
> if (metadata_size) {
> + int bits_left = s->gb.size_in_bits - get_bits_count(&s->gb);
> + if(((bits_left + 7) >> 3) < metadata_size) {
> + skip_bits_long(&s->gb, bits_left);
> + break;
> + }
> switch (metadata_type) {
> case METADATA_TYPE_STREAMINFO:
> + if(streaminfo_updated || metadata_size != FLAC_STREAMINFO_SIZE) {
> + metadata_last = 1;
> + break;
> + }
> metadata_streaminfo(s);
> streaminfo_updated = 1;
> break;
>
> default:
> - for (i=0; i<metadata_size; i++)
> - skip_bits(&s->gb, 8);
> + skip_bits_long(&s->gb, 8*metadata_size);
> }
what does this change do in this patch?!
this is either a cosmetic change (or if metadata_size<0 its wrong)
> + } else if(metadata_type == METADATA_TYPE_STREAMINFO) {
> + metadata_last = 1;
> }
> } while (!metadata_last);
>
> - if (streaminfo_updated)
> + if (streaminfo_updated) {
> + /* check for invalid values in streaminfo */
> + if(s->min_blocksize < 16 || s->max_blocksize < 16) {
> + av_log(s->avctx, AV_LOG_DEBUG, "invalid block size. must be >= 16.\n");
> + return 0;
> + }
> + if(!s->samplerate) {
> + av_log(s->avctx, AV_LOG_DEBUG, "invalid sample rate. must be > 0.\n");
> + return 0;
> + }
if it must be >0 then check for it being <=0 !
> + if(s->bps < 4) {
> + av_log(s->avctx, AV_LOG_DEBUG, "invalid bits-per-sample. must be >= 4.\n");
> + return 0;
> + }
> allocate_buffers(s);
> + }
patch rejected, you cant just skip reallocating
the buffers, the return of this function is not checked nothing will
stop the randomly sized buffers from being used
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Frequently ignored awnser#1 FFmpeg bugs should be sent to our bugtracker, user
questions for the command line tools ffmpeg, ffplay, ... as well as questions
about how to use libav* should be sent to the ffmpeg-user mailinglist.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071002/d1ed8774/attachment.pgp>
More information about the ffmpeg-devel
mailing list