[FFmpeg-devel] [PATCH] Fix mpeg1/2 scantable overflow

Loune Lam lpgcritter at nasquan.com
Thu Aug 9 15:54:56 CEST 2012


Hi All,

Recently been trying VLC on android, and discovered a crash with a 
corrupted DVB mpeg2 ts video. Traced it down to libavcodec/mpeg12.c, 
Snippet mpeg2_fast_decode_block_intra and Snippet 
mpeg2_fast_decode_block_non_intra. Basically they index scantable 
without checking if the bounds. The indexable range maximum seems to be 
63 so doing a > 63 check fixes the crash. I've got no idea about how 
mpeg decodes, so not sure if that fix is the best. Also, there seems to 
be a few other methods (i.e. mpeg1/non-fast) which suffer from this same 
mistake of not validating the index when accessing the scantable. Some 
of them do have a (i > 63) check (error ac-tex damaged at...) but it's 
after accessing the scantable. I've added a range check to all places 
which looks at the scantable.

cheers,

-l
-------------- next part --------------
>From 2e7a544b963b447add3137fbec889181c15a3f81 Mon Sep 17 00:00:00 2001
From: Loune Lam <lpgcritter at nasquan.com>
Date: Thu, 9 Aug 2012 23:12:42 +1000
Subject: [PATCH] Fix mpeg1/2 scantable overflow

Signed-off-by: Loune Lam <lpgcritter at nasquan.com>
---
 libavcodec/mpeg12.c |   85 +++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 63 insertions(+), 22 deletions(-)

diff --git a/libavcodec/mpeg12.c b/libavcodec/mpeg12.c
index 1bf857e..ee57550 100644
--- a/libavcodec/mpeg12.c
+++ b/libavcodec/mpeg12.c
@@ -112,6 +112,10 @@ static inline int mpeg1_decode_block_intra(MpegEncContext *s, DCTELEM *block, in
                 break;
             } else if (level != 0) {
                 i += run;
+                if (i > 63) {
+                    av_log(s->avctx, AV_LOG_ERROR, "ac-tex damaged at %d %d\n", s->mb_x, s->mb_y);
+                    return -1;
+                }
                 j = scantable[i];
                 level = (level * qscale * quant_matrix[j]) >> 4;
                 level = (level - 1) | 1;
@@ -128,6 +132,10 @@ static inline int mpeg1_decode_block_intra(MpegEncContext *s, DCTELEM *block, in
                     level = SHOW_UBITS(re, &s->gb, 8)      ; LAST_SKIP_BITS(re, &s->gb, 8);
                 }
                 i += run;
+                if (i > 63) {
+                    av_log(s->avctx, AV_LOG_ERROR, "ac-tex damaged at %d %d\n", s->mb_x, s->mb_y);
+                    return -1;
+                }
                 j = scantable[i];
                 if (level < 0) {
                     level = -level;
@@ -139,10 +147,6 @@ static inline int mpeg1_decode_block_intra(MpegEncContext *s, DCTELEM *block, in
                     level = (level - 1) | 1;
                 }
             }
-            if (i > 63) {
-                av_log(s->avctx, AV_LOG_ERROR, "ac-tex damaged at %d %d\n", s->mb_x, s->mb_y);
-                return -1;
-            }
 
             block[j] = level;
         }
@@ -187,6 +191,10 @@ static inline int mpeg1_decode_block_inter(MpegEncContext *s, DCTELEM *block, in
 
             if (level != 0) {
                 i += run;
+                if (i > 63) {
+                    av_log(s->avctx, AV_LOG_ERROR, "ac-tex damaged at %d %d\n", s->mb_x, s->mb_y);
+                    return -1;
+                }
                 j = scantable[i];
                 level = ((level * 2 + 1) * qscale * quant_matrix[j]) >> 5;
                 level = (level - 1) | 1;
@@ -203,6 +211,10 @@ static inline int mpeg1_decode_block_inter(MpegEncContext *s, DCTELEM *block, in
                     level = SHOW_UBITS(re, &s->gb, 8)      ; SKIP_BITS(re, &s->gb, 8);
                 }
                 i += run;
+                if (i > 63) {
+                    av_log(s->avctx, AV_LOG_ERROR, "ac-tex damaged at %d %d\n", s->mb_x, s->mb_y);
+                    return -1;
+                }
                 j = scantable[i];
                 if (level < 0) {
                     level = -level;
@@ -214,10 +226,6 @@ static inline int mpeg1_decode_block_inter(MpegEncContext *s, DCTELEM *block, in
                     level = (level - 1) | 1;
                 }
             }
-            if (i > 63) {
-                av_log(s->avctx, AV_LOG_ERROR, "ac-tex damaged at %d %d\n", s->mb_x, s->mb_y);
-                return -1;
-            }
 
             block[j] = level;
             if (((int32_t)GET_CACHE(re, &s->gb)) <= (int32_t)0xBFFFFFFF)
@@ -262,6 +270,10 @@ static inline int mpeg1_fast_decode_block_inter(MpegEncContext *s, DCTELEM *bloc
 
             if (level != 0) {
                 i += run;
+                if (i > 63) {
+                    av_log(s->avctx, AV_LOG_ERROR, "ac-tex damaged at %d %d\n", s->mb_x, s->mb_y);
+                    return -1;
+                }
                 j = scantable[i];
                 level = ((level * 2 + 1) * qscale) >> 1;
                 level = (level - 1) | 1;
@@ -278,6 +290,10 @@ static inline int mpeg1_fast_decode_block_inter(MpegEncContext *s, DCTELEM *bloc
                     level = SHOW_UBITS(re, &s->gb, 8)      ; SKIP_BITS(re, &s->gb, 8);
                 }
                 i += run;
+                if (i > 63) {
+                    av_log(s->avctx, AV_LOG_ERROR, "ac-tex damaged at %d %d\n", s->mb_x, s->mb_y);
+                    return -1;
+                }
                 j = scantable[i];
                 if (level < 0) {
                     level = -level;
@@ -343,6 +359,10 @@ static inline int mpeg2_decode_block_non_intra(MpegEncContext *s, DCTELEM *block
 
             if (level != 0) {
                 i += run;
+                if (i > 63) {
+                    av_log(s->avctx, AV_LOG_ERROR, "ac-tex damaged at %d %d\n", s->mb_x, s->mb_y);
+                    return -1;
+                }
                 j = scantable[i];
                 level = ((level * 2 + 1) * qscale * quant_matrix[j]) >> 5;
                 level = (level ^ SHOW_SBITS(re, &s->gb, 1)) - SHOW_SBITS(re, &s->gb, 1);
@@ -354,6 +374,10 @@ static inline int mpeg2_decode_block_non_intra(MpegEncContext *s, DCTELEM *block
                 level = SHOW_SBITS(re, &s->gb, 12); SKIP_BITS(re, &s->gb, 12);
 
                 i += run;
+                if (i > 63) {
+                    av_log(s->avctx, AV_LOG_ERROR, "ac-tex damaged at %d %d\n", s->mb_x, s->mb_y);
+                    return -1;
+                }
                 j = scantable[i];
                 if (level < 0) {
                     level = ((-level * 2 + 1) * qscale * quant_matrix[j]) >> 5;
@@ -362,10 +386,6 @@ static inline int mpeg2_decode_block_non_intra(MpegEncContext *s, DCTELEM *block
                     level = ((level * 2 + 1) * qscale * quant_matrix[j]) >> 5;
                 }
             }
-            if (i > 63) {
-                av_log(s->avctx, AV_LOG_ERROR, "ac-tex damaged at %d %d\n", s->mb_x, s->mb_y);
-                return -1;
-            }
 
             mismatch ^= level;
             block[j]  = level;
@@ -412,6 +432,10 @@ static inline int mpeg2_fast_decode_block_non_intra(MpegEncContext *s,
 
         if (level != 0) {
             i += run;
+            if (i > 63) {
+                av_log(s->avctx, AV_LOG_ERROR, "ac-tex damaged at %d %d\n", s->mb_x, s->mb_y);
+                return -1;
+            }
             j  = scantable[i];
             level = ((level * 2 + 1) * qscale) >> 1;
             level = (level ^ SHOW_SBITS(re, &s->gb, 1)) - SHOW_SBITS(re, &s->gb, 1);
@@ -423,6 +447,10 @@ static inline int mpeg2_fast_decode_block_non_intra(MpegEncContext *s,
             level = SHOW_SBITS(re, &s->gb, 12); SKIP_BITS(re, &s->gb, 12);
 
             i += run;
+            if (i > 63) {
+                av_log(s->avctx, AV_LOG_ERROR, "ac-tex damaged at %d %d\n", s->mb_x, s->mb_y);
+                return -1;
+            }
             j  = scantable[i];
             if (level < 0) {
                 level = ((-level * 2 + 1) * qscale) >> 1;
@@ -489,6 +517,10 @@ static inline int mpeg2_decode_block_intra(MpegEncContext *s, DCTELEM *block, in
                 break;
             } else if (level != 0) {
                 i += run;
+                if (i > 63) {
+                    av_log(s->avctx, AV_LOG_ERROR, "ac-tex damaged at %d %d\n", s->mb_x, s->mb_y);
+                    return -1;
+                }
                 j  = scantable[i];
                 level = (level * qscale * quant_matrix[j]) >> 4;
                 level = (level ^ SHOW_SBITS(re, &s->gb, 1)) - SHOW_SBITS(re, &s->gb, 1);
@@ -499,6 +531,10 @@ static inline int mpeg2_decode_block_intra(MpegEncContext *s, DCTELEM *block, in
                 UPDATE_CACHE(re, &s->gb);
                 level = SHOW_SBITS(re, &s->gb, 12); SKIP_BITS(re, &s->gb, 12);
                 i += run;
+                if (i > 63) {
+                    av_log(s->avctx, AV_LOG_ERROR, "ac-tex damaged at %d %d\n", s->mb_x, s->mb_y);
+                    return -1;
+                }
                 j  = scantable[i];
                 if (level < 0) {
                     level = (-level * qscale * quant_matrix[j]) >> 4;
@@ -507,10 +543,6 @@ static inline int mpeg2_decode_block_intra(MpegEncContext *s, DCTELEM *block, in
                     level = (level * qscale * quant_matrix[j]) >> 4;
                 }
             }
-            if (i > 63) {
-                av_log(s->avctx, AV_LOG_ERROR, "ac-tex damaged at %d %d\n", s->mb_x, s->mb_y);
-                return -1;
-            }
 
             mismatch ^= level;
             block[j]  = level;
@@ -525,7 +557,7 @@ static inline int mpeg2_decode_block_intra(MpegEncContext *s, DCTELEM *block, in
 
 static inline int mpeg2_fast_decode_block_intra(MpegEncContext *s, DCTELEM *block, int n)
 {
-    int level, dc, diff, j, run;
+    int level, dc, diff, j, run, i;
     int component;
     RLTable *rl;
     uint8_t * scantable = s->intra_scantable.permutated;
@@ -551,6 +583,7 @@ static inline int mpeg2_fast_decode_block_intra(MpegEncContext *s, DCTELEM *bloc
         rl = &ff_rl_mpeg2;
     else
         rl = &ff_rl_mpeg1;
+    i = 0;
 
     {
         OPEN_READER(re, &s->gb);
@@ -562,8 +595,12 @@ static inline int mpeg2_fast_decode_block_intra(MpegEncContext *s, DCTELEM *bloc
             if (level == 127) {
                 break;
             } else if (level != 0) {
-                scantable += run;
-                j = *scantable;
+                i += run;
+                if (i > 63) {
+                    av_log(s->avctx, AV_LOG_ERROR, "ac-tex damaged at %d %d\n", s->mb_x, s->mb_y);
+                    return -1;
+                }
+                j = scantable[i];
                 level = (level * qscale * quant_matrix[j]) >> 4;
                 level = (level ^ SHOW_SBITS(re, &s->gb, 1)) - SHOW_SBITS(re, &s->gb, 1);
                 LAST_SKIP_BITS(re, &s->gb, 1);
@@ -572,8 +609,12 @@ static inline int mpeg2_fast_decode_block_intra(MpegEncContext *s, DCTELEM *bloc
                 run = SHOW_UBITS(re, &s->gb, 6) + 1; LAST_SKIP_BITS(re, &s->gb, 6);
                 UPDATE_CACHE(re, &s->gb);
                 level = SHOW_SBITS(re, &s->gb, 12); SKIP_BITS(re, &s->gb, 12);
-                scantable += run;
-                j = *scantable;
+                i += run;
+                if (i > 63) {
+                    av_log(s->avctx, AV_LOG_ERROR, "ac-tex damaged at %d %d\n", s->mb_x, s->mb_y);
+                    return -1;
+                }
+                j = scantable[i];
                 if (level < 0) {
                     level = (-level * qscale * quant_matrix[j]) >> 4;
                     level = -level;
@@ -587,7 +628,7 @@ static inline int mpeg2_fast_decode_block_intra(MpegEncContext *s, DCTELEM *bloc
         CLOSE_READER(re, &s->gb);
     }
 
-    s->block_last_index[n] = scantable - s->intra_scantable.permutated;
+    s->block_last_index[n] = i;
     return 0;
 }
 
-- 
1.7.8.6



More information about the ffmpeg-devel mailing list