[FFmpeg-devel] [PATCH] ff_split_xiph_headers returns broken header_len < 0

[Michael Niedermayer]

On Fri, Apr 18, 2008 at 11:02:56PM +0200, Reimar D?ffinger wrote:
> Hello,
> On Tue, Apr 15, 2008 at 06:54:45PM +0200, Reimar D?ffinger wrote:
> > when trying to play http://wdz5.xs4all.nl/~hendrik/mmw-deadzy.ogg with
> > MPlayer (ffplay untested), the vorbis decoder crashes.
> > The reason is that ff_split_xiph_headers does not fail but returns an
> > invalid (negative) header_len[2].
> > Attached patch is one possible fix. Maybe doing a return -1 if this case
> > happens is the better solution, I just like to default to solutions that
> > have a higher chance of still working with broken files (though at least
> > in this case it does not help anyway, the files till does not play).
> 
> That code is rather annoyingly full of problems and I am a bit confused,
> but I think attached patch should be quite ok.
> 
> Greetings,
> Reimar D?ffinger

> Index: libavcodec/xiph.c
> ===================================================================
> --- libavcodec/xiph.c	(revision 12879)
> +++ libavcodec/xiph.c	(working copy)
> @@ -26,25 +26,31 @@
>  {
>      int i, j;
>  
> -    if (AV_RB16(extradata) == first_header_size) {
> +    if (extradata_size >= 6 && AV_RB16(extradata) == first_header_size) {
> +        int overall_len = 6;
>          for (i=0; i<3; i++) {
>              header_len[i] = AV_RB16(extradata);
>              extradata += 2;
>              header_start[i] = extradata;
>              extradata += header_len[i];
> +            if (overall_len > extradata_size - header_len[i])
> +                return -1;
> +            overall_len += header_len[i];

>          }
> -    } else if (extradata[0] == 2) {
> +    } else if (extradata_size >= 3 && extradata_size + 0x1ff > 0 && extradata[0] == 2) {

this check will not work with gcc, have i not been loud enough about gccs
behaviour with signed overflow checks?


[..]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080419/fa2c15ce/attachment.pgp>