[FFmpeg-cvslog] r19973 - trunk/libavcodec/utils.c

[Uoti Urpala]

On Wed, 2009-09-23 at 00:44 +0200, michael wrote:
> Log:
> Check codec_id and codec_type in avcodec_open(), based on 43_codec_type_mismatch.patch from chrome
> This is said to be able to lead to a stack based buffer overflow.
> 
> Modified:
>    trunk/libavcodec/utils.c
> 
> Modified: trunk/libavcodec/utils.c
> ==============================================================================
> --- trunk/libavcodec/utils.c	Tue Sep 22 22:38:03 2009	(r19972)
> +++ trunk/libavcodec/utils.c	Wed Sep 23 00:44:56 2009	(r19973)
> @@ -481,7 +481,10 @@ int attribute_align_arg avcodec_open(AVC
>      }
>  
>      avctx->codec = codec;
> -    avctx->codec_id = codec->id;
> +    if(avctx->codec_id != codec->id || avctx->codec_type != codec->type){
> +        av_log(avctx, AV_LOG_ERROR, "codec type or id mismatches\n");
> +        goto end;
> +    }

What's the point of this? Is the application supposed to set those
before calling avcodec_open()? If so then why couldn't FFmpeg set them
just as well instead of checking they're already set? Or what kind of
usage is assumed where they could already be set to different values and
checking it could be meaningful? Is this about FFmpeg itself using
avcodec_open() internally in an unsafe way?

At least MPlayer does not set avctx->codec_id or avctx->codec_type
before calling avcodec_open() and so wouldn't work with this change. Of
course setting them would be trivial, but does requiring that really
make sense for the API?