Re: [FFmpeg-devel] [PATCH] smacker: limit recursion depth of smacker_decode_bigtree

[Andreas Cadhalpun]

On 19.11.2016 16:13, Michael Niedermayer wrote:
> On Sat, Nov 19, 2016 at 02:29:35PM +0100, Andreas Cadhalpun wrote:
>> This fixes segmentation faults due to stack-overflow caused by too deep
>> recursion.
>>
>> Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
>> ---
>>  libavcodec/smacker.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> breaks fate
> 
> [smackvid @ 0x3586b80] size 31232 too large
> Input #0, smk, from 'fate/fate-suite//smacker/wetlogo.smk':
>   Duration: 00:00:07.10, start: 0.000000, bitrate: 815 kb/s
>     Stream #0:0: Video: smackvideo (SMK2 / 0x324B4D53), pal8, 320x200, 14.08 
> tbr, 14.08 tbn, 14.08 tbc
>     Stream #0:1: Audio: smackaudio (SMKA / 0x414B4D53), 22050 Hz, mono, u8
> [smackvid @ 0x358b860] size 31232 too large
> Stream mapping:
>   Stream #0:0 -> #0:0 (smackvideo (smackvid) -> rawvideo (native))
> Error while opening decoder for input stream #0:0 : Invalid data found when 
> processing input
> make: *** [fate-smacker-video] Error 1

Then this has to be checked more directly, see attached patch.

Best regards,
Andreas


>From bcb4a860ff5ca987b5bcd99797704faaba6d23a2 Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
Date: Sat, 19 Nov 2016 14:21:11 +0100
Subject: [PATCH] smacker: limit recursion depth of smacker_decode_bigtree

This fixes segmentation faults due to stack-overflow caused by too deep
recursion.

Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
---
 libavcodec/smacker.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/libavcodec/smacker.c b/libavcodec/smacker.c
index b8a0c55..be3914b 100644
--- a/libavcodec/smacker.c
+++ b/libavcodec/smacker.c
@@ -129,8 +129,12 @@ static int smacker_decode_tree(GetBitContext *gb, HuffContext *hc, uint32_t pref
 /**
  * Decode header tree
  */
-static int smacker_decode_bigtree(GetBitContext *gb, HuffContext *hc, DBCtx *ctx)
+static int smacker_decode_bigtree(GetBitContext *gb, HuffContext *hc, DBCtx *ctx, int length)
 {
+    if(length > 5000) { // Larger length can cause segmentation faults due to too deep recursion.
+        av_log(NULL, AV_LOG_ERROR, "length too long\n");
+        return AVERROR_INVALIDDATA;
+    }
     if (hc->current + 1 >= hc->length) {
         av_log(NULL, AV_LOG_ERROR, "Tree size exceeded!\n");
         return AVERROR_INVALIDDATA;
@@ -159,12 +163,12 @@ static int smacker_decode_bigtree(GetBitContext *gb, HuffContext *hc, DBCtx *ctx
         int r = 0, r_new, t;
 
         t = hc->current++;
-        r = smacker_decode_bigtree(gb, hc, ctx);
+        r = smacker_decode_bigtree(gb, hc, ctx, length + 1);
         if(r < 0)
             return r;
         hc->values[t] = SMK_NODE | r;
         r++;
-        r_new = smacker_decode_bigtree(gb, hc, ctx);
+        r_new = smacker_decode_bigtree(gb, hc, ctx, length + 1);
         if (r_new < 0)
             return r_new;
         return r + r_new;
@@ -275,7 +279,7 @@ static int smacker_decode_header_tree(SmackVContext *smk, GetBitContext *gb, int
         goto error;
     }
 
-    if (smacker_decode_bigtree(gb, &huff, &ctx) < 0)
+    if (smacker_decode_bigtree(gb, &huff, &ctx, 0) < 0)
         err = -1;
     skip_bits1(gb);
     if(ctx.last[0] == -1) ctx.last[0] = huff.current++;
-- 
2.10.2


_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel