> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of
> James Almer
> Sent: Thursday, 30 September 2021 05:53
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v5 3/7] libavformat/asfdec: Fix
> type of value_len
>
> On 9/29/2021 11:58 PM, Soft Works wrote:
> > The value_len is an uint32 not an int32 per spec. That
> > value must not be truncated, neither by casting to int, nor by any
> > conditional checks, because at the end of get_tag, this value is
> > needed to move forward in parsing. When the len value gets
> > modified, the parsing may break.
> >
> > Signed-off-by: softworkz <softwo...@hotmail.com>
> > ---
> > v5: Split into pieces as requested
> >
> > libavformat/asfdec_f.c | 24 +++++++++++-------------
> > 1 file changed, 11 insertions(+), 13 deletions(-)
> >
> > diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
> > index 076b5ab147..d017fae019 100644
> > --- a/libavformat/asfdec_f.c
> > +++ b/libavformat/asfdec_f.c
> > @@ -218,7 +218,7 @@ static uint64_t get_value(AVIOContext *pb, int
> type, int type2_size)
> > }
> > }
> >
> > -static void get_tag(AVFormatContext *s, const char *key, int type,
> int len, int type2_size)
> > +static void get_tag(AVFormatContext *s, const char *key, int type,
> uint32_t len, int type2_size)
>
> There's an av_assert0() in this function to ensure len will never be
> bigger than (INT_MAX - 22) / 2. And len is used as argument for
> avio_read(), which takes an int, not an unsigned int. So this change
> is
> not ok.
The general situation is this:
The ASF spec allows sizes of various elements in its format up to a
maximum of UINT32_MAX.
As you have correctly noted: we can't process those sizes because
some of the internal functions are taking int32 parameters rather than
uint32. (e.g. with avio_read, get_id3_tag, asf_read_picture, ...)
This patch can't fix that part. But what it fixes is that such values
between INT32_MAX and UINT32_MAX won't break ASF parsing anymore.
We emit a message that there's some metadata value that we can't
read, but we skip over it and continue parsing.
That's one part of what this patch does.
Kind regards,
softworkz
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
