Re: [FFmpeg-devel] [PATCH] libopenjpegenc: fix out-of-bounds reads when filling the edges

[Andreas Cadhalpun]

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

>From 1461064c1eaabb71661f9ff68b94f35a1b98e3b5 Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun <andreas.cadhal...@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.cadhal...@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


_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel