[FFmpeg-devel] [PATCH 2/3] cafdec: prevent overreading the info chunk

[Anton Khirnov]

On Thu, Mar 03, 2011 at 11:18:38AM -0500, Ronald S. Bultje wrote:
> Hi,
> 
> On Thu, Mar 3, 2011 at 8:04 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Thu, Mar 03, 2011 at 01:51:56PM +0100, Anton Khirnov wrote:
> >> ---
> >> ?libavformat/cafdec.c | ? ?5 +++--
> >> ?1 files changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/libavformat/cafdec.c b/libavformat/cafdec.c
> >> index d98c4bf..715dfdf 100644
> >> --- a/libavformat/cafdec.c
> >> +++ b/libavformat/cafdec.c
> >> @@ -182,11 +182,12 @@ static void read_info_chunk(AVFormatContext *s, int64_t size)
> >> ? ? ?AVIOContext *pb = s->pb;
> >> ? ? ?unsigned int i;
> >> ? ? ?unsigned int nb_entries = avio_rb32(pb);
> >> + ? ?size -= 4;
> >> ? ? ?for (i = 0; i < nb_entries; i++) {
> >> ? ? ? ? ?char key[32];
> >> ? ? ? ? ?char value[1024];
> >> - ? ? ? ?get_strz(pb, key, sizeof(key));
> >> - ? ? ? ?get_strz(pb, value, sizeof(value));
> >> + ? ? ? ?size -= avio_get_str(pb, size, key, ? sizeof(key));
> >> + ? ? ? ?size -= avio_get_str(pb, size, value, sizeof(value));
> >> ? ? ? ? ?av_metadata_set2(&s->metadata, key, value, 0);
> >> ? ? ?}
> >> ?}
> >
> > I really dont belive that for the case where this makes a difference to the
> > current code that this would be the correct way to handle it
> >
> > 2 strings of 100 bytes in a 102 bytes chunk leads to the second string to be
> > 2 bytes long and silently used without error message
> 
> That's a broken file, nothing more.
> 
> The question is whether this patch increases the chance of parsing the
> rest of the file correctly. There isn't really a "correct" answer to
> that question. It might in some cases, it might not in others,
> depending on how the file is/was damaged. Most likely it will fail
> before and will still fail after this patch, just differently.
> 

Ok, whatever, I don't care about this demuxer and don't want to waste
more time bikeshedding over it. New code should behave the same as the
old in the attached patch.

-- 
Anton Khirnov
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavf-deprecate-get_strz-in-favor-of-avio_get_str.patch
Type: text/x-diff
Size: 3673 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110303/d136bd59/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110303/d136bd59/attachment.pgp>