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

[Michael Niedermayer]

On Sat, Nov 19, 2016 at 05:27:19PM +0100, Andreas Cadhalpun wrote:
> 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.Cadhalpun at 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
> 

>  smacker.c |   12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 91ae1418a6828a849e6239bd127e76d3a4ba700f  0001-smacker-limit-recursion-depth-of-smacker_decode_bigt.patch
> From bcb4a860ff5ca987b5bcd99797704faaba6d23a2 Mon Sep 17 00:00:00 2001
> From: Andreas Cadhalpun <Andreas.Cadhalpun at 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.Cadhalpun at 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;
> +    }

are you sure this is not too large for some platforms ?

patch ok otherwise

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have often repented speaking, but never of holding my tongue.
-- Xenocrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20161119/6dc2440d/attachment.sig>