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

[Reimar Döffinger]

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
-------------- next part --------------
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) {
+        int overall_len = 3;
         for (i=0,j=1; i<2; i++,j++) {
             header_len[i] = 0;
-            for (; j<extradata_size && extradata[j]==0xff; j++) {
+            for (; overall_len < extradata_size && extradata[j]==0xff; j++) {
                 header_len[i] += 0xff;
+                overall_len   += 0xff + 1;
             }
-            if (j >= extradata_size)
+            header_len[i] += extradata[j];
+            overall_len   += extradata[j];
+            if (overall_len > extradata_size)
                 return -1;
-
-            header_len[i] += extradata[j];
         }
-        header_len[2] = extradata_size - header_len[0] - header_len[1] - j;
+        header_len[2] = extradata_size - overall_len;
         extradata += j;
         header_start[0] = extradata;
         header_start[1] = header_start[0] + header_len[0];