[FFmpeg-devel] [PATCH v7 1/2] libavcodec/pgxdec: Add PGX decoder

[Michael Niedermayer]

On Sat, Jul 04, 2020 at 06:45:02PM +0200, Nicolas George wrote:
> Michael Niedermayer (12020-07-04):
[...]
> > > > +    if (bytestream2_get_bytes_left(&g) < width * height * (bpp >> 3))
> > > The multiplication can overflow.
> > ff_set_dimensions() should prevent this
> 
> No, it should not, because it is not aware of the specifics. The default
> value for max_pixels is INT_MAX, but it is only a default value,
> applications or users can set it to a higher value. More importantly,
> ff_set_dimensions() does not know about bpp >> 3, doesn't know that the
> whole frame needs to fit in the size of AVPacket.

Currently
av_image_check_size2() checks

if ((int)w<=0 || (int)h<=0 || stride >= INT_MAX || stride*(uint64_t)(h+128) >= INT_MAX) {

That uses INT_MAX and stride tries to include the maximum pixel size
av_image_check_size2() could be used directly and the actual pixel format
could be passed to it but it is still redundant and we dont do this in many
other places.
Also it wouldnt make a difference as ff_set_dimensions uses AV_PIX_FMT_NONE
and would fail with resolutions such a av_image_check_size2() would succeeed

If the goal would be to make ff_set_dimensions() work with cases beyond
INT_MAX addressible then we should extend ff_set_dimensions() itself
because requiring a seperate function to check this is dangerous as it
can be forgotten. 
Also increasing the limit for all uses of ff_set_dimensions() is in itself
also dangerous

Its most robust is if the function setting the dimensions does the check
already
we could add a internal codec cap which disables the INT_MAX or a 
ff_set_dimensions2() all this is a little outside the scope of this
patch review though

there also may have been patches changing the INT_MAX limit, dont
remember exactly

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Never trust a computer, one day, it may think you are the virus. -- Compn
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200704/ca499711/attachment.sig>