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

[Andreas Cadhalpun]

On 20.11.2016 00:44, Andreas Cadhalpun wrote:
> On 19.11.2016 23:34, Michael Niedermayer wrote:
>> On Sat, Nov 19, 2016 at 05:27:19PM +0100, Andreas Cadhalpun wrote:
>>> 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 ?
> 
> I don't think it's even possible to make this small enough for all cases,
> as the stack size can be arbitrarily changed with 'ulimit -s'.
> 
> This value was chosen so that it works with the default stack size of 8 MB,
> but if you think that's too much, it can be made smaller.
> 
> Attached is a variant reducing the 5000 to 500 and thus still working
> with a stack size of only 0.8 MB.

I've now pushed this variant with reduced stack size needs.

Best regards,
Andreas