[FFmpeg-devel] [PATCH] avutil/imgutils: av_image_check_size2() ensure width and height fit in 32bit

Anton Khirnov anton at khirnov.net
Wed Jul 10 11:23:57 EEST 2024


Quoting Michael Niedermayer (2024-07-10 00:00:32)
> On Tue, Jul 09, 2024 at 05:14:37PM +0200, Anton Khirnov wrote:
> > Quoting Michael Niedermayer (2024-07-09 15:28:10)
> > > On Tue, Jul 09, 2024 at 03:17:58PM +0200, Anton Khirnov wrote:
> > > > > ensure width and height fit in 32bit
> > > > 
> > > > why?
> > > 
> > > because not everyone wants undefined behavior
> > > because not everyone wants security issues
> > > because we dont support width and height > 32bit and its easier to check in a central place
> > > because the changed codes purpose is to check if the image paramaters are
> > >     within what we support, and width of 100 billion is not. You can try
> > >     all encoders with 100billion width. Then try to decode.
> > >     Iam curious, how many work, how many fail and how they fail
> > >     how many invalid bitstreams with no warning, how many undefined behaviors, ...
> > > 
> > > Simply building FFmpeg on a platform with 64bit ints doesnt update
> > > ISO and ITU standards to allow larger values
> > 
> > Quoting Michael Niedermayer (2020-10-07 16:45:56):
> > > At least in code i wrote and write i consider it a bug if it would
> > > assume sizeof(int/unsigned) == 4
> > 
> > Make up your mind.
> 
> Where do you see a contradiction ?
> 
> 2020: assuming sizeof(int/unsigned) == 4 is a bug
> 2024: we do not support more than 32bit width and height,
>       nor is that supported by the majority of codec bitsterams and formats
>       -> We thus should in a central place check that instead of generating
>       undefined behavior and security issues
> 
> What i suggest IS actually fixing a "sizeof(int/unsigned) == 4" bug
> 
> If someone wants to make the codebase work with 64bit width and height, this
> should not be limited to "int is 64bit" systems that would be a very seriously
> broken design and also very illogic.

I don't see any existing conditions on video dimensions being 32bit, the
condition currently is that every dimension and their product must fit
in an int. I also don't see what actual problems does this patch
address.

> Also your terse replies feel a bit rude

What a coincidence, your wall of patronizing blah blah that does nothing to
actually answer my original question also seems pretty rude to me. Reap
what you sow.

-- 
Anton Khirnov


More information about the ffmpeg-devel mailing list