[FFmpeg-devel] [PATCH 2/3] mlvdec: validate bits_per_coded_sample

[Andreas Cadhalpun]

On 20.12.2015 00:55, Michael Niedermayer wrote:
> On Sat, Dec 19, 2015 at 11:49:02PM +0100, Andreas Cadhalpun wrote:
>> A negative bits_per_coded_sample doesn't make sense.
>> If it is too large, the size calculation for av_get_packet overflows,
>> resulting in allocation of a too small buffer.
>>
>> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
>> ---
>>  libavformat/mlvdec.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/libavformat/mlvdec.c b/libavformat/mlvdec.c
>> index 4b3bdc1..2e57aae 100644
>> --- a/libavformat/mlvdec.c
>> +++ b/libavformat/mlvdec.c
>> @@ -135,6 +135,15 @@ static int scan_file(AVFormatContext *avctx, AVStream *vst, AVStream *ast, int f
>>                  avpriv_request_sample(avctx, "raw api version");
>>              avio_skip(pb, 20); // pointer, width, height, pitch, frame_size
>>              vst->codec->bits_per_coded_sample = avio_rl32(pb);
>> +            if (vst->codec->bits_per_coded_sample < 0 ||
>> +                (vst->codec->width && vst->codec->height &&
> 
>> +                vst->codec->bits_per_coded_sample > (INT_MAX - 7) / (vst->codec->width * vst->codec->height))) {
> 
> w*h can overflow

OK, but that should be checked via av_image_check_size.
Updated patch attached.

> might be easier to calculate it in unsigned 64bit and then check

av_image_check_size does it correctly.

> the value also could be reused to ensure it wont get out of sync with
> the allocation

If width or height could get out of sync, so could the precomputed value.
So I don't think reusing the value is very useful here.

Best regards,
Andreas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-mlvdec-validate-bits_per_coded_sample.patch
Type: text/x-diff
Size: 2277 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151220/0a3470ca/attachment.patch>