[FFmpeg-devel] [PATCH] jpegls: allocate large enough zero buffer

[Michael Niedermayer]

On Thu, Dec 31, 2015 at 05:02:14PM +0100, Andreas Cadhalpun wrote:
> On 30.12.2015 21:12, Andreas Cadhalpun wrote:
> > It is read up to length s->width * stride, which can be larger than the
> > linesize. (stride = (s->nb_components > 1) ? 3 : 1)
> > 
> > This fixes an out of bounds read.
> > 
> > Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> > ---
> >  libavcodec/jpeglsdec.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libavcodec/jpeglsdec.c b/libavcodec/jpeglsdec.c
> > index 68151cb..11ffe93 100644
> > --- a/libavcodec/jpeglsdec.c
> > +++ b/libavcodec/jpeglsdec.c
> > @@ -348,7 +348,7 @@ int ff_jpegls_decode_picture(MJpegDecodeContext *s, int near,
> >      JLSState *state;
> >      int off = 0, stride = 1, width, shift, ret = 0;
> >  
> > -    zero = av_mallocz(s->picture_ptr->linesize[0]);
> > +    zero = av_mallocz(FFMAX(s->picture_ptr->linesize[0], s->width * ((s->nb_components > 1) ? 3 : 1)));
> >      if (!zero)
> >          return AVERROR(ENOMEM);
> >      last = zero;
> > 
> 
> A better fix is to error out before this happens.
> Patch doing that attached.
> 
> Best regards,
> Andreas

>  mjpegdec.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> e4b9f65abd49be0714b6367f8530d1829102e6d8  0001-mjpegdec-extend-check-for-incompatible-values-of-s-r.patch
> From 637a849f80bff4acaa42afe8cb4d2dd60fc4248a Mon Sep 17 00:00:00 2001
> From: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> Date: Thu, 31 Dec 2015 16:55:43 +0100
> Subject: [PATCH] mjpegdec: extend check for incompatible values of s->rgb and
>  s->ls
> 
> This can happen if s->ls changes from 0 to 1, but picture allocation is
> skipped due to s->interlaced.
> 
> In that case ff_jpegls_decode_picture could be called even though the
> s->picture_ptr frame has the wrong pixel format and thus a wrong
> linesize, which results in a too small zero buffer being allocated.
> 
> This fixes an out-of-bounds read in ls_decode_line.
> 
> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> ---
>  libavcodec/mjpegdec.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
> index c812b86..c730e05 100644
> --- a/libavcodec/mjpegdec.c
> +++ b/libavcodec/mjpegdec.c
> @@ -632,7 +632,8 @@ unk_pixfmt:
>          av_log(s->avctx, AV_LOG_DEBUG, "decode_sof0: error, len(%d) mismatch\n", len);
>      }
>  
> -    if (s->rgb && !s->lossless && !s->ls) {
> +    if ((s->rgb && !s->lossless && !s->ls) ||
> +        (!s->rgb && s->ls && s->nb_components > 1)) {
>          av_log(s->avctx, AV_LOG_ERROR, "Unsupported coding and pixel format combination\n");
>          return AVERROR_PATCHWELCOME;

LGTM

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 1
"Used only once"    - "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."
-------------- 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/20151231/2860a8a2/attachment.sig>