[FFmpeg-devel] [libav-devel] Re: [PATCH] apedec: ensure blockstodecode is large enough

[Andreas Cadhalpun]

On 28.04.2015 14:35, Luca Barbato wrote:
> On 27/04/15 23:56, Andreas Cadhalpun wrote:
>> s->decoded_buffer is allocated with a min_size of:
>>     2 * FFALIGN(blockstodecode, 8) * sizeof(*s->decoded_buffer)
>>
>> Then it is assigned to s->decoded[0], which is passed as out buffer to
>> decode_array_0000.
>>
>> In this function 64 elements of the out buffer are written
>> unconditionally and outside the array if blocksdecode is too small.
>>
>> This causes memory corruption, leading to segmentation faults or other crashes.
>>
>> Thus check that FFALIGN(blockstodecode, 8) is at least 32, i. e. the
>> decoded_buffer has at least 64 components.
>>
>> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
>> ---
>>  libavcodec/apedec.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
>> index 536361c..06f3d3f 100644
>> --- a/libavcodec/apedec.c
>> +++ b/libavcodec/apedec.c
>> @@ -1481,6 +1481,12 @@ static int ape_decode_frame(AVCodecContext *avctx, void *data,
>>      if (s->fileversion < 3930)
>>          blockstodecode = s->samples;
>>  
>> +    if (FFALIGN(blockstodecode, 8) < 32) {
>> +        av_log(avctx, AV_LOG_ERROR, "Too few blocks to decode %d (< 32)\n",
>> +               FFALIGN(blockstodecode, 8));
>> +        return AVERROR_INVALIDDATA;
>> +    }
>> +
>>      /* reallocate decoded sample buffer if needed */
>>      av_fast_malloc(&s->decoded_buffer, &s->decoded_size,
>>                     2 * FFALIGN(blockstodecode, 8) * sizeof(*s->decoded_buffer));
>>
> 
> I'd just error out for `(s->fileversion < 3860 && nblocks <
> MIN_BLOCKS_0000)`, that function needs at least 64 blocks.

I think it's better to change the function not to need 64 blocks,
as in the second patch I sent.

Best regards,
Andreas