[FFmpeg-devel] [PATCH] rmdec.c: correctly skip indexes

Michael Niedermayer michaelni
Tue Dec 30 20:31:29 CET 2008


On Tue, Dec 30, 2008 at 02:07:31PM -0500, Ronald S. Bultje wrote:
> Hi,
> 
> see $subj. Current code only skips the index header, not the actual
> index content, resulting in a few packets with random content at the
> end of a .rm file playback. This patch fixes that.
> 
> Ronald

> Index: ffmpeg-svn/libavformat/rmdec.c
> ===================================================================
> --- ffmpeg-svn.orig/libavformat/rmdec.c	2008-12-30 13:44:01.000000000 -0500
> +++ ffmpeg-svn/libavformat/rmdec.c	2008-12-30 14:03:38.000000000 -0500
> @@ -445,9 +445,11 @@
>              state= (state<<8) + get_byte(pb);
>  
>              if(state == MKBETAG('I', 'N', 'D', 'X')){
> -                len = get_be16(pb) - 6;
> -                if(len<0)
> -                    continue;
> +                int size, n_pkts;
> +                size = get_be32(pb);
> +                url_fskip(pb, 4);
> +                n_pkts = get_be16(pb);
> +                len = size - 14 + n_pkts * 14;
>                  goto skip;
>              }

you silently remove a validity check, do you just not give a damn if the
code is exploitable or do you not read your patches at all before submitting?
seriously, please look at your changes before submitting, i likely do catch
such obvious issues but theres a very good chance i would miss less obvious
issues in code i do not know well (like RT(S)P)
File data can never be trusted, so please think about if there are any
values that can cause out of array reads, writes or an infinite loop by
seeking back.
This one can likely here cause
1. an infinite loop be seeking back
2. a out of array write in rm_assemble_video_frame() by setting remaining_len
   to INT_MAX

besides you change a 16bit BE to a 32bit BE read, its hard to imagine how
both could have made sense, you should explain this assuming your code
does make any sense.


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

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch
-------------- 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/20081230/380e3e66/attachment.pgp>



More information about the ffmpeg-devel mailing list