Am So., 8. Aug. 2021 um 03:00 Uhr schrieb Soft Works <softwo...@hotmail.com>: > > > -----Original Message----- > > From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of > > James Almer > > Sent: Sunday, 8 August 2021 02:47 > > To: ffmpeg-devel@ffmpeg.org > > Subject: Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix regression > > bug when reading image attachments > > > > On 8/7/2021 9:38 PM, Soft Works wrote: > > >> -----Original Message----- > > >> From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of > > >> Carl Eugen Hoyos > > >> Sent: Sunday, 8 August 2021 01:58 > > >> To: FFmpeg development discussions and patches <ffmpeg- > > >> de...@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 > > >> <softwo...@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 <softwo...@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 :-) > > > > len < (INT_MAX - LEN) / 2 is more than half the range of an int. > > I've been one step ahead in calculation: > > av_malloc takes size_t and on 32bit platforms, this is uint32, which means > that it can take a maximum of 4.294.967.295. > (4.294.967.295 - 22) / 2 = 2.147.483.636 (+ 11 makes INT32_MAX) > > > > > > > > > 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. > > > > The assert is to ensure get_tag() is not called with out of range values, > > so the > > relevant checks should happen before the call. > > I'm not sure whether I'd agree with that. It shouldn't be the caller > needing to know what the function can handle and what it can't handle.
> The INT32/2 is not only wrong, it also does _only_ apply to reading > unicode. For ASCII and byte arrays, there is no need for this restriction. > And on 64bit platforms, there's no need for this that restriction at all. There is: An allocation >32bit for an attachment in an asf file is always wrong. Carl Eugen _______________________________________________ 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".