[FFmpeg-devel] [PATCH 2/4] avformat/mxfdec: Check count in mxf_read_strong_ref_array()

[Michael Niedermayer]

On Sun, Mar 20, 2022 at 02:05:41PM +0100, Tomas Härdin wrote:
> lör 2022-03-19 klockan 23:50 +0100 skrev Michael Niedermayer:
[...]
> > 
> > 
> > > 
> > > > +
> > > > +    //avio_read() used int
> > > > +    if (c > INT_MAX / sizeof(UID))
> > > > +        return AVERROR_PATCHWELCOME;
> > > > +    *count = c;
> > > > +
> > > 
> > > This should already be caught by av_calloc(), no?
> > 
> > the API as in the documentation of av_calloc() does not gurantee
> > this. 
> 
> Yes it does:
> 
>   The allocated memory will have size `size * nmemb` bytes.
>   [...]
>   `NULL` if the block cannot be allocated

void *av_calloc(size_t nmemb, size_t size)
size_t can be larger than int, so size * nmemb may be larger than INT_MAX


> 
> > Its bad practice if we write code that depends on some implementation
> > of some code in a diferent module/lib
> 
> If av_calloc() does not guarantee this then it is useless. It is used
> precisely for this all over the place. Are you going to change every
> use of av_calloc() in mxfdec in the same way?

well, when max_alloc_size is set above INT_MAX 
then int checks will become needed when these values ever get stored in
ints. For example here avio_read() has a int argument that is set to the
product of the 2. Or all such ints need to be changed to something bigger

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato
-------------- 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/20220320/e4711fb3/attachment.sig>