[FFmpeg-devel] [RFC] ignore invalid user-supplied width/height

[Reimar Döffinger]

On Thu, Sep 02, 2010 at 09:44:43PM +0200, Michael Niedermayer wrote:
> On Thu, Sep 02, 2010 at 09:14:04PM +0200, Reimar D?ffinger wrote:
> > On Thu, Sep 02, 2010 at 11:04:22AM +0200, Michael Niedermayer wrote:
> > > On Tue, Aug 31, 2010 at 09:49:33PM +0200, Reimar D?ffinger wrote:
> > > > most video codecs will figure out a width/height themselves or fail
> > > > if they can't.
> > > > So IMO it is better not to fail for invalid values in avcodec_open but
> > > > instead just ignore the values by using the "default" of 0.
> > > > Otherwise applications would have to manually check the values with
> > > > av_check_image_size if they want the video to remain playable even
> > > > if the container values were corrupted.
> > > > Any objections?
> > > 
> > > yes, this change will leave invalid values in width/height and has a
> > > good chance that this may be exploitable with some decoder
> > 
> > Yes, that was quite silly.
> > Any other comment?
> 
> diff with -p next time please :)               

Any idea how to add this permanently to the SVN config?

> and avcodec_set_dimensions(0,0) could be used and maybe that can be simplified
> with the surrounding code, would have to see it first to be sure if so ...

I forgot that avcodec_set_dimensions can be used.
I think I'm too tired, but I think there might already be an issue as-is,
if an application set
codec_width = coded_height = width = 0,
height = 0x7fffffff
that would be passed on to the codec as-is.
I made another try, but as said too tired.

Index: libavcodec/utils.c
===================================================================
--- libavcodec/utils.c  (revision 25017)
+++ libavcodec/utils.c  (working copy)
@@ -485,10 +485,15 @@ int attribute_align_arg avcodec_open(AVCodecContex
     else if(avctx->width && avctx->height)
         avcodec_set_dimensions(avctx, avctx->width, avctx->height);
 
+    if ((avctx->coded_width || avctx->coded_height || avctx->width || avctx->height)
+        && (  av_check_image_size(avctx->coded_width, avctx->coded_height, 0, avctx) < 0
+           || av_check_image_size(avctx->width,       avctx->height,       0, avctx) < 0) {
+        av_log(avctx, AV_LOG_WARNING, "ignoring invalid width/height values\n");
+        avcodec_set_dimensions(avctx, 0, 0);
+    }
+
 #define SANE_NB_CHANNELS 128U
-    if (((avctx->coded_width || avctx->coded_height)
-        && av_check_image_size(avctx->coded_width, avctx->coded_height, 0, avctx))
-        || avctx->channels > SANE_NB_CHANNELS) {
+    if (avctx->channels > SANE_NB_CHANNELS) {
         ret = AVERROR(EINVAL);
         goto free_and_end;
     }