[FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix regression bug when reading image attachments

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Carl Eugen Hoyos
> Sent: Sunday, 8 August 2021 01:58
> To: FFmpeg development discussions and patches <ffmpeg-
> devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix regression
> bug when reading image attachments
> 
> Am So., 8. Aug. 2021 um 01:53 Uhr schrieb Soft Works
> <softworkz at hotmail.com>:
> >
> > Commit c8140fe7324f264faacf7395b27e12531d1f13f7 had introduced a
> check for value_len > UINT16_MAX.
> > As a consequence, attached images of sizes larger than UINT16_MAX could
> no longer be read.
> >
> > Signed-off-by: softworkz <softworkz at hotmail.com>
> > ---
> > v2: Fix without changing variable type  libavformat/asfdec_f.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c index
> > ff6ddfb967..b9f3918495 100644
> > --- a/libavformat/asfdec_f.c
> > +++ b/libavformat/asfdec_f.c
> > @@ -614,7 +614,7 @@ static int asf_read_metadata(AVFormatContext *s,
> int64_t size)
> >          value_type = avio_rl16(pb); /* value_type */
> >          value_len  = avio_rl32(pb);
> >
> > -        if (value_len < 0 || value_len > UINT16_MAX)
> > +        if (value_len < 0)
> >              return AVERROR_INVALIDDATA;
> 
> I may misread the code but it appears to me that an assert can be triggered
> now, no?

Right, for these 11 discrete size values only, though:

2147483647, 2147483646, 2147483645, 2147483644, 2147483643,
2147483642, 2147483641, 2147483640, 2147483639, 2147483638
2147483636

A chance of 11 in 4 Billions :-)

TBH, I don't understand this assert. When the attachment is too large
to handle, we should rather log a warning and goto finish instead IMO.

softworkz