CVE-2016-3183: Out-of-bounds Read (CWE-125)
value: The sycc422_t_rgb function in common/color.c in OpenJPEG before 2.1.1 allows remote attackers to cause a denial of service (out-of-bounds read) via a crafted jpeg2000 file.
CWE description: The product reads data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 5.5
Published: 2017-02-03T16:59Z
Repository: https://github.com/uclouvain/openjpeg
Language: C
Files changed: src/bin/common/color.c, src/bin/jp2/convertbmp.c
Commit date: 2016-04-30 00:33:27 +0200
Commit message: Fix Out-Of-Bounds Read in sycc42x_to_rgb function (#745)

42x Images with an odd x0/y0 lead to subsampled component starting at the
2nd column/line.
That is offset = comp->dx * comp->x0 - image->x0 = 1

Fix #726
Diff: diff --git a/src/bin/common/color.c b/src/bin/common/color.c
index 00eac515..20aba2ba 100644
--- a/src/bin/common/color.c
+++ b/src/bin/common/color.c
@@ -89,26 +89,26 @@ static void sycc_to_rgb(int offset, int upb, int y, int cb, int cr,
 
 static void sycc444_to_rgb(opj_image_t *img)
 {
 	int *d0, *d1, *d2, *r, *g, *b;
 	const int *y, *cb, *cr;
-	unsigned int maxw, maxh, max, i;
+	size_t maxw, maxh, max, i;
 	int offset, upb;
 
 	upb = (int)img->comps[0].prec;
 	offset = 1<<(upb - 1); upb = (1<<upb)-1;
 
-	maxw = (unsigned int)img->comps[0].w; maxh = (unsigned int)img->comps[0].h;
+	maxw = (size_t)img->comps[0].w; maxh = (size_t)img->comps[0].h;
 	max = maxw * maxh;
 
 	y = img->comps[0].data;
 	cb = img->comps[1].data;
 	cr = img->comps[2].data;
 
-	d0 = r = (int*)malloc(sizeof(int) * (size_t)max);
-	d1 = g = (int*)malloc(sizeof(int) * (size_t)max);
-	d2 = b = (int*)malloc(sizeof(int) * (size_t)max);
+	d0 = r = (int*)malloc(sizeof(int) * max);
+	d1 = g = (int*)malloc(sizeof(int) * max);
+	d2 = b = (int*)malloc(sizeof(int) * max);
 
 	if(r == NULL || g == NULL || b == NULL) goto fails;
 
 	for(i = 0U; i < max; ++i)
 	{
@@ -116,111 +116,142 @@ static void sycc444_to_rgb(opj_image_t *img)
 		++y; ++cb; ++cr; ++r; ++g; ++b;
 	}
 	free(img->comps[0].data); img->comps[0].data = d0;
 	free(img->comps[1].data); img->comps[1].data = d1;
 	free(img->comps[2].data); img->comps[2].data = d2;
+	img->color_space = OPJ_CLRSPC_SRGB;
 	return;
 
 fails:
-	if(r) free(r);
-	if(g) free(g);
-	if(b) free(b);
-
+	free(r);
+	free(g);
+	free(b);
 }/* sycc444_to_rgb() */
 
 static void sycc422_to_rgb(opj_image_t *img)
 {	
 	int *d0, *d1, *d2, *r, *g, *b;
 	const int *y, *cb, *cr;
-	unsigned int maxw, maxh, max;
+	size_t maxw, maxh, max, offx, loopmaxw;
 	int offset, upb;
-	unsigned int i, j;
+	size_t i;
 
 	upb = (int)img->comps[0].prec;
 	offset = 1<<(upb - 1); upb = (1<<upb)-1;
 
-	maxw = (unsigned int)img->comps[0].w; maxh = (unsigned int)img->comps[0].h;
+	maxw = (size_t)img->comps[0].w; maxh = (size_t)img->comps[0].h;
 	max = maxw * maxh;
 
 	y = img->comps[0].data;
 	cb = img->comps[1].data;
 	cr = img->comps[2].data;
 
-	d0 = r = (int*)malloc(sizeof(int) * (size_t)max);
-	d1 = g = (int*)malloc(sizeof(int) * (size_t)max);
-	d2 = b = (int*)malloc(sizeof(int) * (size_t)max);
+	d0 = r = (int*)malloc(sizeof(int) * max);
+	d1 = g = (int*)malloc(sizeof(int) * max);
+	d2 = b = (int*)malloc(sizeof(int) * max);
 
 	if(r == NULL || g == NULL || b == NULL) goto fails;
 
+	/* if img->x0 is odd, then first column shall use Cb/Cr = 0 */
+	offx = img->x0 & 1U;
+	loopmaxw = maxw - offx;
+	
 	for(i=0U; i < maxh; ++i)
 	{
-		for(j=0U; j < (maxw & ~(unsigned int)1U); j += 2U)
+		size_t j;
+		
+		if (offx > 0U) {
+			sycc_to_rgb(offset, upb, *y, 0, 0, r, g, b);
+			++y; ++r; ++g; ++b;
+		}
+		
+		for(j=0U; j < (loopmaxw & ~(size_t)1U); j += 2U)
 		{
 			sycc_to_rgb(offset, upb, *y, *cb, *cr, r, g, b);
 			++y; ++r; ++g; ++b;
 			sycc_to_rgb(offset, upb, *y, *cb, *cr, r, g, b);
 			++y; ++r; ++g; ++b; ++cb; ++cr;
 		}
-		if (j < maxw) {
+		if (j < loopmaxw) {
 			sycc_to_rgb(offset, upb, *y, *cb, *cr, r, g, b);
 			++y; ++r; ++g; ++b; ++cb; ++cr;
 		}
 	}
+	
 	free(img->comps[0].data); img->comps[0].data = d0;
 	free(img->comps[1].data); img->comps[1].data = d1;
 	free(img->comps[2].data); img->comps[2].data = d2;
 
-#if defined(USE_JPWL) || defined(USE_MJ2)
-	img->comps[1].w = maxw; img->comps[1].h = maxh;
-	img->comps[2].w = maxw; img->comps[2].h = maxh;
-#else
-	img->comps[1].w = (OPJ_UINT32)maxw; img->comps[1].h = (OPJ_UINT32)maxh;
-	img->comps[2].w = (OPJ_UINT32)maxw; img->comps[2].h = (OPJ_UINT32)maxh;
-#endif
-	img->comps[1].dx = img->comps[0].dx;
-	img->comps[2].dx = img->comps[0].dx;
-	img->comps[1].dy = img->comps[0].dy;
-	img->comps[2].dy = img->comps[0].dy;
+	img->comps[1].w = img->comps[2].w = img->comps[0].w;
+	img->comps[1].h = img->comps[2].h = img->comps[0].h;
+	img->comps[1].dx = img->comps[2].dx = img->comps[0].dx;
+	img->comps[1].dy = img->comps[2].dy = img->comps[0].dy;
+	img->color_space = OPJ_CLRSPC_SRGB;
 	return;
 
 fails:
-	if(r) free(r);
-	if(g) free(g);
-	if(b) free(b);
-
+	free(r);
+	free(g);
+	free(b);
 }/* sycc422_to_rgb() */
 
 static void sycc420_to_rgb(opj_image_t *img)
 {
 	int *d0, *d1, *d2, *r, *g, *b, *nr, *ng, *nb;
 	const int *y, *cb, *cr, *ny;
-	unsigned int maxw, maxh, max;
+	size_t maxw, maxh, max, offx, loopmaxw, offy, loopmaxh;
 	int offset, upb;
-	unsigned int i, j;
+	size_t i;
 
 	upb = (int)img->comps[0].prec;
 	offset = 1<<(upb - 1); upb = (1<<upb)-1;
 
-	maxw = (unsigned int)img->comps[0].w; maxh = (unsigned int)img->comps[0].h;
+	maxw = (size_t)img->comps[0].w; maxh = (size_t)img->comps[0].h;
 	max = maxw * maxh;
 
 	y = img->comps[0].data;
 	cb = img->comps[1].data;
 	cr = img->comps[2].data;
 
-	d0 = r = (int*)malloc(sizeof(int) * (size_t)max);
-	d1 = g = (int*)malloc(sizeof(int) * (size_t)max);
-	d2 = b = (int*)malloc(sizeof(int) * (size_t)max);
-
-	if(r == NULL || g == NULL || b == NULL) goto fails;
+	d0 = r = (int*)malloc(sizeof(int) * max);
+	d1 = g = (int*)malloc(sizeof(int) * max);
+	d2 = b = (int*)malloc(sizeof(int) * max);
+	
+	if (r == NULL || g == NULL || b == NULL) goto fails;
+	
+	/* if img->x0 is odd, then first column shall use Cb/Cr = 0 */
+	offx = img->x0 & 1U;
+	loopmaxw = maxw - offx;
+	/* if img->y0 is odd, then first line shall use Cb/Cr = 0 */
+	offy = img->y0 & 1U;
+	loopmaxh = maxh - offy;
+	
+	if (offy > 0U) {
+		size_t j;
+		
+		for(j=0; j < maxw; ++j)
+		{
+			sycc_to_rgb(offset, upb, *y, 0, 0, r, g, b);
+			++y; ++r; ++g; ++b;
+		}
+	}
 
-	for(i=0U; i < (maxh & ~(unsigned int)1U); i += 2U)
+	for(i=0U; i < (loopmaxh & ~(size_t)1U); i += 2U)
 	{
+		size_t j;
+		
 		ny = y + maxw;
 		nr = r + maxw; ng = g + maxw; nb = b + maxw;
+		
+		if (offx > 0U) {
+			sycc_to_rgb(offset, upb, *y, 0, 0, r, g, b);
+			++y; ++r; ++g; ++b;
+			sycc_to_rgb(offset, upb, *ny, *cb, *cr, nr, ng, nb);
+			++ny; ++nr; ++ng; ++nb;
+		}
 
-		for(j=0; j < (maxw & ~(unsigned int)1U); j += 2U)
+		for(j=0; j < (loopmaxw & ~(size_t)1U); j += 2U)
 		{
 			sycc_to_rgb(offset, upb, *y, *cb, *cr, r, g, b);
 			++y; ++r; ++g; ++b;
 			sycc_to_rgb(offset, upb, *y, *cb, *cr, r, g, b);
 			++y; ++r; ++g; ++b;
@@ -228,23 +259,25 @@ static void sycc420_to_rgb(opj_image_t *img)
 			sycc_to_rgb(offset, upb, *ny, *cb, *cr, nr, ng, nb);
 			++ny; ++nr; ++ng; ++nb;
 			sycc_to_rgb(offset, upb, *ny, *cb, *cr, nr, ng, nb);
 			++ny; ++nr; ++ng; ++nb; ++cb; ++cr;
 		}
-		if(j < maxw)
+		if(j < loopmaxw)
 		{
 			sycc_to_rgb(offset, upb, *y, *cb, *cr, r, g, b);
 			++y; ++r; ++g; ++b;
 
 			sycc_to_rgb(offset, upb, *ny, *cb, *cr, nr, ng, nb);
 			++ny; ++nr; ++ng; ++nb; ++cb; ++cr;
 		}
 		y += maxw; r += maxw; g += maxw; b += maxw;
 	}
-	if(i < maxh)
+	if(i < loopmaxh)
 	{
-		for(j=0U; j < (maxw & ~(unsigned int)1U); j += 2U)
+		size_t j;
+		
+		for(j=0U; j < (maxw & ~(size_t)1U); j += 2U)
 		{
 			sycc_to_rgb(offset, upb, *y, *cb, *cr, r, g, b);
 
 			++y; ++r; ++g; ++b;
 
@@ -260,28 +293,21 @@ static void sycc420_to_rgb(opj_image_t *img)
 
 	free(img->comps[0].data); img->comps[0].data = d0;
 	free(img->comps[1].data); img->comps[1].data = d1;
 	free(img->comps[2].data); img->comps[2].data = d2;
 
-#if defined(USE_JPWL) || defined(USE_MJ2)
-	img->comps[1].w = maxw; img->comps[1].h = maxh;
-	img->comps[2].w = maxw; img->comps[2].h = maxh;
-#else
-	img->comps[1].w = (OPJ_UINT32)maxw; img->comps[1].h = (OPJ_UINT32)maxh;
-	img->comps[2].w = (OPJ_UINT32)maxw; img->comps[2].h = (OPJ_UINT32)maxh;
-#endif
-	img->comps[1].dx = img->comps[0].dx;
-	img->comps[2].dx = img->comps[0].dx;
-	img->comps[1].dy = img->comps[0].dy;
-	img->comps[2].dy = img->comps[0].dy;
+	img->comps[1].w = img->comps[2].w = img->comps[0].w;
+	img->comps[1].h = img->comps[2].h = img->comps[0].h;
+	img->comps[1].dx = img->comps[2].dx = img->comps[0].dx;
+	img->comps[1].dy = img->comps[2].dy = img->comps[0].dy;
+	img->color_space = OPJ_CLRSPC_SRGB;
 	return;
 
 fails:
-	if(r) free(r);
-	if(g) free(g);
-	if(b) free(b);
-
+	free(r);
+	free(g);
+	free(b);
 }/* sycc420_to_rgb() */
 
 void color_sycc_to_rgb(opj_image_t *img)
 {
 	if(img->numcomps < 3)
@@ -322,12 +348,10 @@ void color_sycc_to_rgb(opj_image_t *img)
 	else
   {
 		fprintf(stderr,"%s:%d:color_sycc_to_rgb\n\tCAN NOT CONVERT\n", __FILE__,__LINE__);
 		return;
   }
-	img->color_space = OPJ_CLRSPC_SRGB;
-
 }/* color_sycc_to_rgb() */
 
 #if defined(OPJ_HAVE_LIBLCMS2) || defined(OPJ_HAVE_LIBLCMS1)
 
 #ifdef OPJ_HAVE_LIBLCMS1
diff --git a/src/bin/jp2/convertbmp.c b/src/bin/jp2/convertbmp.c
index a09c0ae5..d264823f 100644
--- a/src/bin/jp2/convertbmp.c
+++ b/src/bin/jp2/convertbmp.c
@@ -965,11 +965,11 @@ int imagetobmp(opj_image_t * image, const char *outfile) {
             if(r > 255) r = 255; else if(r < 0) r = 0;
 
             fprintf(fdest, "%c", (OPJ_UINT8)r);
 
             if ((i + 1) % w == 0) {
-                for ((pad = w % 4) ? (4 - w % 4) : 0; pad > 0; pad--)	/* ADD */
+                for (pad = (w % 4) ? (4 - w % 4) : 0; pad > 0; pad--)	/* ADD */
                     fprintf(fdest, "%c", 0);
             }
         }
         fclose(fdest);
     }

Vulnerable code: 	unsigned int maxw, maxh, max, i;
	maxw = (unsigned int)img->comps[0].w; maxh = (unsigned int)img->comps[0].h;
	d0 = r = (int*)malloc(sizeof(int) * (size_t)max);
	d1 = g = (int*)malloc(sizeof(int) * (size_t)max);
	d2 = b = (int*)malloc(sizeof(int) * (size_t)max);
	if(r) free(r);
	if(g) free(g);
	if(b) free(b);

	unsigned int maxw, maxh, max;
	unsigned int i, j;
	maxw = (unsigned int)img->comps[0].w; maxh = (unsigned int)img->comps[0].h;
	d0 = r = (int*)malloc(sizeof(int) * (size_t)max);
	d1 = g = (int*)malloc(sizeof(int) * (size_t)max);
	d2 = b = (int*)malloc(sizeof(int) * (size_t)max);
		for(j=0U; j < (maxw & ~(unsigned int)1U); j += 2U)
		if (j < maxw) {
#if defined(USE_JPWL) || defined(USE_MJ2)
	img->comps[1].w = maxw; img->comps[1].h = maxh;
	img->comps[2].w = maxw; img->comps[2].h = maxh;
#else
	img->comps[1].w = (OPJ_UINT32)maxw; img->comps[1].h = (OPJ_UINT32)maxh;
	img->comps[2].w = (OPJ_UINT32)maxw; img->comps[2].h = (OPJ_UINT32)maxh;
#endif
	img->comps[1].dx = img->comps[0].dx;
	img->comps[2].dx = img->comps[0].dx;
	img->comps[1].dy = img->comps[0].dy;
	img->comps[2].dy = img->comps[0].dy;
	if(r) free(r);
	if(g) free(g);
	if(b) free(b);

	unsigned int maxw, maxh, max;
	unsigned int i, j;
	maxw = (unsigned int)img->comps[0].w; maxh = (unsigned int)img->comps[0].h;
	d0 = r = (int*)malloc(sizeof(int) * (size_t)max);
	d1 = g = (int*)malloc(sizeof(int) * (size_t)max);
	d2 = b = (int*)malloc(sizeof(int) * (size_t)max);

	if(r == NULL || g == NULL || b == NULL) goto fails;
	for(i=0U; i < (maxh & ~(unsigned int)1U); i += 2U)
		for(j=0; j < (maxw & ~(unsigned int)1U); j += 2U)
		if(j < maxw)
	if(i < maxh)
		for(j=0U; j < (maxw & ~(unsigned int)1U); j += 2U)
#if defined(USE_JPWL) || defined(USE_MJ2)
	img->comps[1].w = maxw; img->comps[1].h = maxh;
	img->comps[2].w = maxw; img->comps[2].h = maxh;
#else
	img->comps[1].w = (OPJ_UINT32)maxw; img->comps[1].h = (OPJ_UINT32)maxh;
	img->comps[2].w = (OPJ_UINT32)maxw; img->comps[2].h = (OPJ_UINT32)maxh;
#endif
	img->comps[1].dx = img->comps[0].dx;
	img->comps[2].dx = img->comps[0].dx;
	img->comps[1].dy = img->comps[0].dy;
	img->comps[2].dy = img->comps[0].dy;
	if(r) free(r);
	if(g) free(g);
	if(b) free(b);

	img->color_space = OPJ_CLRSPC_SRGB;

                for ((pad = w % 4) ? (4 - w % 4) : 0; pad > 0; pad--)	/* ADD */
Fixed code: 	size_t maxw, maxh, max, i;
	maxw = (size_t)img->comps[0].w; maxh = (size_t)img->comps[0].h;
	d0 = r = (int*)malloc(sizeof(int) * max);
	d1 = g = (int*)malloc(sizeof(int) * max);
	d2 = b = (int*)malloc(sizeof(int) * max);
	img->color_space = OPJ_CLRSPC_SRGB;
	free(r);
	free(g);
	free(b);
	size_t maxw, maxh, max, offx, loopmaxw;
	size_t i;
	maxw = (size_t)img->comps[0].w; maxh = (size_t)img->comps[0].h;
	d0 = r = (int*)malloc(sizeof(int) * max);
	d1 = g = (int*)malloc(sizeof(int) * max);
	d2 = b = (int*)malloc(sizeof(int) * max);
	/* if img->x0 is odd, then first column shall use Cb/Cr = 0 */
	offx = img->x0 & 1U;
	loopmaxw = maxw - offx;
	
		size_t j;
		
		if (offx > 0U) {
			sycc_to_rgb(offset, upb, *y, 0, 0, r, g, b);
			++y; ++r; ++g; ++b;
		}
		
		for(j=0U; j < (loopmaxw & ~(size_t)1U); j += 2U)
		if (j < loopmaxw) {
	
	img->comps[1].w = img->comps[2].w = img->comps[0].w;
	img->comps[1].h = img->comps[2].h = img->comps[0].h;
	img->comps[1].dx = img->comps[2].dx = img->comps[0].dx;
	img->comps[1].dy = img->comps[2].dy = img->comps[0].dy;
	img->color_space = OPJ_CLRSPC_SRGB;
	free(r);
	free(g);
	free(b);
	size_t maxw, maxh, max, offx, loopmaxw, offy, loopmaxh;
	size_t i;
	maxw = (size_t)img->comps[0].w; maxh = (size_t)img->comps[0].h;
	d0 = r = (int*)malloc(sizeof(int) * max);
	d1 = g = (int*)malloc(sizeof(int) * max);
	d2 = b = (int*)malloc(sizeof(int) * max);
	
	if (r == NULL || g == NULL || b == NULL) goto fails;
	
	/* if img->x0 is odd, then first column shall use Cb/Cr = 0 */
	offx = img->x0 & 1U;
	loopmaxw = maxw - offx;
	/* if img->y0 is odd, then first line shall use Cb/Cr = 0 */
	offy = img->y0 & 1U;
	loopmaxh = maxh - offy;
	
	if (offy > 0U) {
		size_t j;
		
		for(j=0; j < maxw; ++j)
		{
			sycc_to_rgb(offset, upb, *y, 0, 0, r, g, b);
			++y; ++r; ++g; ++b;
		}
	}
	for(i=0U; i < (loopmaxh & ~(size_t)1U); i += 2U)
		size_t j;
		
		
		if (offx > 0U) {
			sycc_to_rgb(offset, upb, *y, 0, 0, r, g, b);
			++y; ++r; ++g; ++b;
			sycc_to_rgb(offset, upb, *ny, *cb, *cr, nr, ng, nb);
			++ny; ++nr; ++ng; ++nb;
		}
		for(j=0; j < (loopmaxw & ~(size_t)1U); j += 2U)
		if(j < loopmaxw)
	if(i < loopmaxh)
		size_t j;
		
		for(j=0U; j < (maxw & ~(size_t)1U); j += 2U)
	img->comps[1].w = img->comps[2].w = img->comps[0].w;
	img->comps[1].h = img->comps[2].h = img->comps[0].h;
	img->comps[1].dx = img->comps[2].dx = img->comps[0].dx;
	img->comps[1].dy = img->comps[2].dy = img->comps[0].dy;
	img->color_space = OPJ_CLRSPC_SRGB;
	free(r);
	free(g);
	free(b);
                for (pad = (w % 4) ? (4 - w % 4) : 0; pad > 0; pad--)	/* ADD */