[FFmpeg-devel] [libav-devel] [PATCH] xwddec: prevent overflow of lsize * avctx->height

[Michael Niedermayer]

On Fri, Dec 18, 2015 at 08:13:06PM +0100, Andreas Cadhalpun wrote:
> On 18.12.2015 20:08, Luca Barbato wrote:
> > On 18/12/15 20:04, Andreas Cadhalpun wrote:
> >> This is used to check if the input buffer is larger enough, so if this
> >> overflows it can cause a false negative leading to a segmentation fault
> >> in bytestream2_get_bufferu.
> >>
> >> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> >> ---
> >>  libavcodec/xwddec.c | 6 ++++++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/libavcodec/xwddec.c b/libavcodec/xwddec.c
> >> index 2febedc..311eeec 100644
> >> --- a/libavcodec/xwddec.c
> >> +++ b/libavcodec/xwddec.c
> >> @@ -127,6 +127,12 @@ static int xwd_decode_frame(AVCodecContext *avctx, void *data,
> >>          return AVERROR_INVALIDDATA;
> >>      }
> >>  
> >> +    if (lsize > UINT_MAX / avctx->height) {
> >> +        av_log(avctx, AV_LOG_ERROR, "lsize %u too large for height %d\n",
> >> +               lsize, avctx->height);
> >> +        return AVERROR_INVALIDDATA;
> >> +    }
> >> +
> >>      if (ncolors > 256) {
> >>          av_log(avctx, AV_LOG_ERROR, "invalid number of entries in colormap\n");
> >>          return AVERROR_INVALIDDATA;
> >>
> > 
> > Should go after the av_image_check_size or you'd have a division by 0.
> 
> Indeed. Updated patch attached.
> 
> Best regards,
> Andreas
> 

>  xwddec.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 0be27d89a669445b523bfdac99884065e3581f3c  0001-xwddec-prevent-overflow-of-lsize-avctx-height.patch
> From fb40616d7b432680b92dc3adc44a5b5d12fac55d Mon Sep 17 00:00:00 2001
> From: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> Date: Fri, 18 Dec 2015 19:28:51 +0100
> Subject: [PATCH] xwddec: prevent overflow of lsize * avctx->height
> 
> This is used to check if the input buffer is larger enough, so if this
> overflows it can cause a false negative leading to a segmentation fault
> in bytestream2_get_bufferu.

cant the addition overflow too in the input buffer check ?
if so then using 64bit in the input buffer check would avoid the
need for a explicit check on lsize

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Complexity theory is the science of finding the exact solution to an
approximation. Benchmarking OTOH is finding an approximation of the exact
-------------- 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/20151219/35821e37/attachment.sig>