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

[Andreas Cadhalpun]

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.

Best regards,
Andreas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-smacker-limit-recursion-depth-of-smacker_decode_bigt.patch
Type: text/x-diff
Size: 2207 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20161120/97766b59/attachment.patch>