[FFmpeg-devel] [PATCH] VP3: check that init_vlc with user-supplied data is successful

[Reimar Döffinger]

Hello,
sample is ogv/smclock.ogv.1.101.ogv from issue 1240.
Attached patch makes decode_init fail if the coded huffman tables are
invalid and thus init_vlc fails.
There remains one huge WTF: although decode_init returns -1 each and
every time, with this command-line
./ffmpeg_g -i crashers/ogv/smclock.ogv.1.101.ogv -f framecrc -
FFmpeg will still call the decode function, obviously causing a crash
since the decoder was never correctly initialized.
stack trace:
==26703== Invalid write of size 1
==26703==    at 0x4A09F98: memset (mc_replace_strmem.c:471)
==26703==    by 0x6B9947: vp3_decode_frame (string3.h:85)
==26703==    by 0x493EEC: avcodec_decode_video2 (utils.c:577)
==26703==    by 0x407226: output_packet (ffmpeg.c:1314)
==26703==    by 0x40A3F3: av_encode (ffmpeg.c:2281)
==26703==    by 0x40ACFB: main (ffmpeg.c:3997)
==26703==  Address 0x0 is not stack'd, malloc'd or (recently) free'd

-------------- next part --------------
Index: vp3.c
===================================================================
--- vp3.c	(revision 19346)
+++ vp3.c	(working copy)
@@ -1781,29 +1788,34 @@
         for (i = 0; i < 16; i++) {
 
             /* DC histograms */
-            init_vlc(&s->dc_vlc[i], 5, 32,
+            if (init_vlc(&s->dc_vlc[i], 5, 32,
                 &s->huffman_table[i][0][1], 4, 2,
-                &s->huffman_table[i][0][0], 4, 2, 0);
+                &s->huffman_table[i][0][0], 4, 2, 0) < 0)
+                goto vlc_fail;
 
             /* group 1 AC histograms */
-            init_vlc(&s->ac_vlc_1[i], 5, 32,
+            if (init_vlc(&s->ac_vlc_1[i], 5, 32,
                 &s->huffman_table[i+16][0][1], 4, 2,
-                &s->huffman_table[i+16][0][0], 4, 2, 0);
+                &s->huffman_table[i+16][0][0], 4, 2, 0) < 0)
+                goto vlc_fail;
 
             /* group 2 AC histograms */
-            init_vlc(&s->ac_vlc_2[i], 5, 32,
+            if (init_vlc(&s->ac_vlc_2[i], 5, 32,
                 &s->huffman_table[i+16*2][0][1], 4, 2,
-                &s->huffman_table[i+16*2][0][0], 4, 2, 0);
+                &s->huffman_table[i+16*2][0][0], 4, 2, 0) < 0)
+                goto vlc_fail;
 
             /* group 3 AC histograms */
-            init_vlc(&s->ac_vlc_3[i], 5, 32,
+            if (init_vlc(&s->ac_vlc_3[i], 5, 32,
                 &s->huffman_table[i+16*3][0][1], 4, 2,
-                &s->huffman_table[i+16*3][0][0], 4, 2, 0);
+                &s->huffman_table[i+16*3][0][0], 4, 2, 0) < 0)
+                goto vlc_fail;
 
             /* group 4 AC histograms */
-            init_vlc(&s->ac_vlc_4[i], 5, 32,
+            if (init_vlc(&s->ac_vlc_4[i], 5, 32,
                 &s->huffman_table[i+16*4][0][1], 4, 2,
-                &s->huffman_table[i+16*4][0][0], 4, 2, 0);
+                &s->huffman_table[i+16*4][0][0], 4, 2, 0) < 0)
+                goto vlc_fail;
         }
     }
 
@@ -1837,6 +1849,10 @@
     }
 
     return 0;
+
+vlc_fail:
+    av_log(avctx, AV_LOG_FATAL, "Invalid huffman table\n");
+    return -1;
 }
 
 /*