[FFmpeg-devel] [PATCH v4 1/1] libavformat/asfdec: A collection of related fixes for asfdec

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Michael Niedermayer
> Sent: Tuesday, 28 September 2021 22:18
> To: FFmpeg development discussions and patches <ffmpeg-
> devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v4 1/1] libavformat/asfdec: A
> collection of related fixes for asfdec
> 
> On Tue, Sep 28, 2021 at 08:10:02PM +0000, Soft Works wrote:
> > Fix 1: 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.
> >
> > Fix 2: 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.
> >
> > Fix 3: get_value had a return type of int, which means that reading
> > QWORDS was broken due to truncation.
> >
> > Fix 4: Parsing of GUID values wasn't implemented
> >
> > Fix 5: In get_tag, the code was adding 22 bytes (in order to allow
> > it to hold 64bit numbers as string) to the value len for creating
> > creating a buffer. This was unnecessarily imposing a
> > size-constraint on the value_len parameter.
> >
> > Fix 6: The code in get_tag, was limiting the maximum value_len to
> > half the size of INT32. This was applied for all value types, even
> > though it is required only in case of ASF_UNICODE, not for any
> > other ones (like ASCII).
> >
> > Fix 7: get_tag was always allocating a buffer regardless of the
> > datatype, even though this isn't required in case of ASF_BYTE_ARRAY
> >
> > Fix 8: The spec allows attachment sizes of up to UINT32_MAX while
> > we can handle only sizes up to INT32_MAX. The debug.assert didn't
> > really address this, and truncating the value_len in calling
> > methods cannot be used as the length value is required in order to
> > continue parsing. I have added a check with log message in
> > ff_asf_handle_byte_array to handle this.
> 
> This probably should be split into multiple self contained patches

I'll happily do when I would get some rough feedback on it :-)

Then I'd know what would be desired or undesired and how's the 
best way to split these..

Thanks,
softworkz