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

[Michael Niedermayer]

On Sun, Dec 20, 2015 at 12:15:17PM +0100, Andreas Cadhalpun wrote:
> 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

>  mlvdec.c |   12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 9870daae0ba6a9c826563645319ee38c694025e8  0002-mlvdec-validate-bits_per_coded_sample.patch
> From 66a3af0c54f0db6b96b0bad7ae7b9bbbd980b830 Mon Sep 17 00:00:00 2001
> From: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> Date: Sat, 19 Dec 2015 23:45:00 +0100
> Subject: [PATCH 2/3] mlvdec: validate bits_per_coded_sample
> 
> 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.
> 
> Also make sure width and height are sane.

no more objections from me

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire
-------------- 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/20151221/2af0ffad/attachment.sig>