[FFmpeg-devel] [PATCH] rmdec.c: prevent zero-length packets

[Ronald S. Bultje]

Hi,

On Tue, Mar 17, 2009 at 8:16 AM, Kostya <kostya.shishkov at gmail.com> wrote:
> On Tue, Mar 17, 2009 at 08:00:17AM -0400, Ronald S. Bultje wrote:
>> I don't want to terminate by return with a negative value, since len=0
>> might just be a random bytesequence that was in the middle of a packet
>> for whatever codec (i.e. wrong sync).
>
> Better add that check to sync - we want correct resyncinc after all.

Hmk, see attached (will apply in 2 goes, one for moving the -12 and
one for the actual < to =< change).

I've looked for a while at adding more length-checks at the bottom of
sync(), e.g.:

RMStream *ast = st->priv_data;
if (ast->audio_framesize) {
    expected_len = ast->coded_framesize * ast->sub_packet_h / 2;
} else if (st->codec->codec_id == CODEC_ID_RA_144) {
    expected_len = RAW_PACKET_SIZE;
}

and maybe you could do a few more like this, e.g. if it's AAC then it
should be at least 3*(AV_RB16(first_two_bytes) + 1)-1, you could
probably figure something for video/ac3 as well. Worth it or too much
trouble?

Ronald
-------------- next part --------------
Index: ffmpeg-svn/libavformat/rmdec.c
===================================================================
--- ffmpeg-svn.orig/libavformat/rmdec.c	2009-03-19 21:23:54.000000000 -0400
+++ ffmpeg-svn/libavformat/rmdec.c	2009-03-19 22:33:31.000000000 -0400
@@ -534,18 +534,15 @@
                 goto skip;
             }
 
-            if(state > (unsigned)0xFFFF || state < 12)
+            if(state > (unsigned)0xFFFF || state <= 12)
                 continue;
-            len=state;
+            len=state - 12;
             state= 0xFFFFFFFF;
 
             num = get_be16(pb);
             *timestamp = get_be32(pb);
             res= get_byte(pb); /* reserved */
             *flags = get_byte(pb); /* flags */
-
-
-            len -= 12;
         }
         for(i=0;i<s->nb_streams;i++) {
             st = s->streams[i];