| Message ID | 91054da1-baa0-ecea-4ee1-cf1e58c0e328@googlemail.com |
|---|---|
| State | Accepted |
| Headers | show |
On Fri, Oct 14, 2016 at 02:00:49AM +0200, Andreas Cadhalpun wrote: > On 14.10.2016 00:49, Michael Niedermayer wrote: > > On Fri, Oct 14, 2016 at 12:23:02AM +0200, Andreas Cadhalpun wrote: > >> The avctx->width/avctx->height is not zero, but libopenjpeg_copy_unpacked8 > >> does: > > > >> width = avctx->width / image->comps[compno].dx; > >> height = avctx->height / image->comps[compno].dy; > > > > this looks wrong to me > > the code in mj2_create_image() looks better: > > cmptparm[i].dx = sub_dx[i]; > > cmptparm[i].dy = sub_dy[i]; > > cmptparm[i].w = (avctx->width + sub_dx[i] - 1) / sub_dx[i]; > > cmptparm[i].h = (avctx->height + sub_dy[i] - 1) / sub_dy[i]; > > Indeed this looks better, so I updated the patch (attached) to change the > calculation of width/height. > > Best regards, > Andreas > libopenjpegenc.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > 17061aee3e88729993c9581f688cbfda01fccaac 0001-libopenjpegenc-fix-out-of-bounds-reads-when-filling-.patch > From 1461064c1eaabb71661f9ff68b94f35a1b98e3b5 Mon Sep 17 00:00:00 2001 > From: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> > Date: Thu, 13 Oct 2016 22:14:46 +0200 > Subject: [PATCH] libopenjpegenc: fix out-of-bounds reads when filling the > edges > > The calculation of width/height should round up, not round down to > prevent setting width or height to 0. > > Also image->comps[compno].w is unsigned (at least in openjpeg2), so the > calculation could silently wrap around without the explicit cast to int. LGTM, iam not libopenjpegenc maintainer though also should be backported thx [...]
On Thu, Oct 13, 2016 at 6:49 PM, Michael Niedermayer <michael@niedermayer.cc > wrote: > > > libopenjpegenc.c | 18 +++++++++--------- > > 1 file changed, 9 insertions(+), 9 deletions(-) > > 17061aee3e88729993c9581f688cbfda01fccaac 0001-libopenjpegenc-fix-out- > of-bounds-reads-when-filling-.patch > > From 1461064c1eaabb71661f9ff68b94f35a1b98e3b5 Mon Sep 17 00:00:00 2001 > > From: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> > > Date: Thu, 13 Oct 2016 22:14:46 +0200 > > Subject: [PATCH] libopenjpegenc: fix out-of-bounds reads when filling the > > edges > > > > The calculation of width/height should round up, not round down to > > prevent setting width or height to 0. > > > > Also image->comps[compno].w is unsigned (at least in openjpeg2), so the > > calculation could silently wrap around without the explicit cast to int. > > LGTM, iam not libopenjpegenc maintainer though Looks good to me too. Please feel free to apply it. Thanks!
On 14.10.2016 06:08, Michael Bradshaw wrote: > On Thu, Oct 13, 2016 at 6:49 PM, Michael Niedermayer <michael@niedermayer.cc >> wrote: >> >>> libopenjpegenc.c | 18 +++++++++--------- >>> 1 file changed, 9 insertions(+), 9 deletions(-) >>> 17061aee3e88729993c9581f688cbfda01fccaac 0001-libopenjpegenc-fix-out- >> of-bounds-reads-when-filling-.patch >>> From 1461064c1eaabb71661f9ff68b94f35a1b98e3b5 Mon Sep 17 00:00:00 2001 >>> From: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> >>> Date: Thu, 13 Oct 2016 22:14:46 +0200 >>> Subject: [PATCH] libopenjpegenc: fix out-of-bounds reads when filling the >>> edges >>> >>> The calculation of width/height should round up, not round down to >>> prevent setting width or height to 0. >>> >>> Also image->comps[compno].w is unsigned (at least in openjpeg2), so the >>> calculation could silently wrap around without the explicit cast to int. >> >> LGTM, iam not libopenjpegenc maintainer though > > > Looks good to me too. Please feel free to apply it. Thanks! Pushed. Best regards, Andreas
From 1461064c1eaabb71661f9ff68b94f35a1b98e3b5 Mon Sep 17 00:00:00 2001 From: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> Date: Thu, 13 Oct 2016 22:14:46 +0200 Subject: [PATCH] libopenjpegenc: fix out-of-bounds reads when filling the edges The calculation of width/height should round up, not round down to prevent setting width or height to 0. Also image->comps[compno].w is unsigned (at least in openjpeg2), so the calculation could silently wrap around without the explicit cast to int. Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> --- libavcodec/libopenjpegenc.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/libavcodec/libopenjpegenc.c b/libavcodec/libopenjpegenc.c index 857ee1a..1b7e168 100644 --- a/libavcodec/libopenjpegenc.c +++ b/libavcodec/libopenjpegenc.c @@ -421,7 +421,7 @@ static int libopenjpeg_copy_packed8(AVCodecContext *avctx, const AVFrame *frame, for (; y < image->comps[compno].h; ++y) { image_line = image->comps[compno].data + y * image->comps[compno].w; for (x = 0; x < image->comps[compno].w; ++x) { - image_line[x] = image_line[x - image->comps[compno].w]; + image_line[x] = image_line[x - (int)image->comps[compno].w]; } } } @@ -461,7 +461,7 @@ static int libopenjpeg_copy_packed12(AVCodecContext *avctx, const AVFrame *frame for (; y < image->comps[compno].h; ++y) { image_line = image->comps[compno].data + y * image->comps[compno].w; for (x = 0; x < image->comps[compno].w; ++x) { - image_line[x] = image_line[x - image->comps[compno].w]; + image_line[x] = image_line[x - (int)image->comps[compno].w]; } } } @@ -501,7 +501,7 @@ static int libopenjpeg_copy_packed16(AVCodecContext *avctx, const AVFrame *frame for (; y < image->comps[compno].h; ++y) { image_line = image->comps[compno].data + y * image->comps[compno].w; for (x = 0; x < image->comps[compno].w; ++x) { - image_line[x] = image_line[x - image->comps[compno].w]; + image_line[x] = image_line[x - (int)image->comps[compno].w]; } } } @@ -528,8 +528,8 @@ static int libopenjpeg_copy_unpacked8(AVCodecContext *avctx, const AVFrame *fram } for (compno = 0; compno < numcomps; ++compno) { - width = avctx->width / image->comps[compno].dx; - height = avctx->height / image->comps[compno].dy; + width = (avctx->width + image->comps[compno].dx - 1) / image->comps[compno].dx; + height = (avctx->height + image->comps[compno].dy - 1) / image->comps[compno].dy; for (y = 0; y < height; ++y) { image_line = image->comps[compno].data + y * image->comps[compno].w; frame_index = y * frame->linesize[compno]; @@ -542,7 +542,7 @@ static int libopenjpeg_copy_unpacked8(AVCodecContext *avctx, const AVFrame *fram for (; y < image->comps[compno].h; ++y) { image_line = image->comps[compno].data + y * image->comps[compno].w; for (x = 0; x < image->comps[compno].w; ++x) { - image_line[x] = image_line[x - image->comps[compno].w]; + image_line[x] = image_line[x - (int)image->comps[compno].w]; } } } @@ -570,8 +570,8 @@ static int libopenjpeg_copy_unpacked16(AVCodecContext *avctx, const AVFrame *fra } for (compno = 0; compno < numcomps; ++compno) { - width = avctx->width / image->comps[compno].dx; - height = avctx->height / image->comps[compno].dy; + width = (avctx->width + image->comps[compno].dx - 1) / image->comps[compno].dx; + height = (avctx->height + image->comps[compno].dy - 1) / image->comps[compno].dy; frame_ptr = (uint16_t *)frame->data[compno]; for (y = 0; y < height; ++y) { image_line = image->comps[compno].data + y * image->comps[compno].w; @@ -585,7 +585,7 @@ static int libopenjpeg_copy_unpacked16(AVCodecContext *avctx, const AVFrame *fra for (; y < image->comps[compno].h; ++y) { image_line = image->comps[compno].data + y * image->comps[compno].w; for (x = 0; x < image->comps[compno].w; ++x) { - image_line[x] = image_line[x - image->comps[compno].w]; + image_line[x] = image_line[x - (int)image->comps[compno].w]; } } } -- 2.9.3