CVE-2016-5157: Improper Restriction of Operations within the Bounds of a Memory Buffer (CWE-119)
value: Heap-based buffer overflow in the opj_dwt_interleave_v function in dwt.c in OpenJPEG, as used in PDFium in Google Chrome before 53.0.2785.89 on Windows and OS X and before 53.0.2785.92 on Linux, allows remote attackers to execute arbitrary code via crafted coordinate values in JPEG 2000 data.
CWE description: The product performs operations on a memory buffer, but it reads from or writes to a memory location outside the buffer's intended boundary. This may result in read or write operations on unexpected memory locations that could be linked to other variables, data structures, or internal program data.
CVSS v3 base score: 8.8
Published: 2016-09-11T10:59Z
Repository: https://github.com/uclouvain/openjpeg
Language: C
Files changed: src/lib/openjp2/tcd.c, tests/compare_dump_files.c, tests/nonregression/test_suite.ctest.in
Commit date: 2016-09-08 00:24:15 +0200
Commit message: Add sanity check for tile coordinates (#823)

Coordinates are casted from OPJ_UINT32 to OPJ_INT32
Add sanity check for negative values and upper bound becoming lower
than lower bound.
See also
https://pdfium.googlesource.com/pdfium/+/b6befb2ed2485a3805cddea86dc7574510178ea9
Diff: diff --git a/src/lib/openjp2/tcd.c b/src/lib/openjp2/tcd.c
index 12da05ca..7a29c491 100644
--- a/src/lib/openjp2/tcd.c
+++ b/src/lib/openjp2/tcd.c
@@ -694,13 +694,24 @@ static INLINE OPJ_BOOL opj_tcd_init_tile(opj_tcd_t *p_tcd, OPJ_UINT32 p_tile_no,
 	
 	/* 4 borders of the tile rescale on the image if necessary */
 	l_tx0 = l_cp->tx0 + p * l_cp->tdx; /* can't be greater than l_image->x1 so won't overflow */
 	l_tile->x0 = (OPJ_INT32)opj_uint_max(l_tx0, l_image->x0);
 	l_tile->x1 = (OPJ_INT32)opj_uint_min(opj_uint_adds(l_tx0, l_cp->tdx), l_image->x1);
+	/* all those OPJ_UINT32 are casted to OPJ_INT32, let's do some sanity check */
+	if ((l_tile->x0 < 0) || (l_tile->x1 <= l_tile->x0)) {
+		opj_event_msg(manager, EVT_ERROR, "Tile X coordinates are not supported\n");
+		return OPJ_FALSE;
+	}
 	l_ty0 = l_cp->ty0 + q * l_cp->tdy; /* can't be greater than l_image->y1 so won't overflow */
 	l_tile->y0 = (OPJ_INT32)opj_uint_max(l_ty0, l_image->y0);
 	l_tile->y1 = (OPJ_INT32)opj_uint_min(opj_uint_adds(l_ty0, l_cp->tdy), l_image->y1);
+	/* all those OPJ_UINT32 are casted to OPJ_INT32, let's do some sanity check */
+	if ((l_tile->y0 < 0) || (l_tile->y1 <= l_tile->y0)) {
+		opj_event_msg(manager, EVT_ERROR, "Tile Y coordinates are not supported\n");
+		return OPJ_FALSE;
+	}
+	
 
 	/* testcase 1888.pdf.asan.35.988 */
 	if (l_tccp->numresolutions == 0) {
 		opj_event_msg(manager, EVT_ERROR, "tiles require at least one resolution\n");
 		return OPJ_FALSE;
diff --git a/tests/compare_dump_files.c b/tests/compare_dump_files.c
index 946c92a5..7d222708 100644
--- a/tests/compare_dump_files.c
+++ b/tests/compare_dump_files.c
@@ -116,14 +116,14 @@ static int parse_cmdline_cmp(int argc, char **argv, test_cmp_parameters* param)
 int main(int argc, char **argv)
 {
   test_cmp_parameters inParam;
   FILE *fbase=NULL, *ftest=NULL;
   int same = 0;
-  char lbase[256];
-  char strbase[256];
-  char ltest[256];
-  char strtest[256];
+  char lbase[512];
+  char strbase[512];
+  char ltest[512];
+  char strtest[512];
 
   if( parse_cmdline_cmp(argc, argv, &inParam) == 1 )
     {
     compare_dump_files_help_display();
     goto cleanup;
@@ -152,13 +152,13 @@ int main(int argc, char **argv)
     }
   printf("Ok.\n");
 
   while (fgets(lbase, sizeof(lbase), fbase) && fgets(ltest,sizeof(ltest),ftest))
     {
-    int nbase = sscanf(lbase, "%255[^\r\n]", strbase);
-    int ntest = sscanf(ltest, "%255[^\r\n]", strtest);
-    assert( nbase != 255 && ntest != 255 );
+    int nbase = sscanf(lbase, "%511[^\r\n]", strbase);
+    int ntest = sscanf(ltest, "%511[^\r\n]", strtest);
+    assert( nbase != 511 && ntest != 511 );
     if( nbase != 1 || ntest != 1 )
       {
       fprintf(stderr, "could not parse line from files\n" );
       goto cleanup;
       }
diff --git a/tests/nonregression/test_suite.ctest.in b/tests/nonregression/test_suite.ctest.in
index bbf1e77f..2dfabfbc 100644
--- a/tests/nonregression/test_suite.ctest.in
+++ b/tests/nonregression/test_suite.ctest.in
@@ -564,5 +564,8 @@ opj_decompress -i @INPUT_NR_PATH@/issue726.j2k -o @TEMP_PATH@/issue726.png
 # issue 775
 !opj_decompress -i @INPUT_NR_PATH@/issue775.j2k -o @TEMP_PATH@/issue775.png
 !opj_decompress -i @INPUT_NR_PATH@/issue775-2.j2k -o @TEMP_PATH@/issue775-2.png
 # issue 818
 opj_decompress -i @INPUT_NR_PATH@/issue818.jp2 -o @TEMP_PATH@/issue818.png
+# issue 823 (yes, not a typo, test image is issue822)
+!opj_decompress -i @INPUT_NR_PATH@/issue822.jp2 -o @TEMP_PATH@/issue822.png
+

Vulnerable code:   char lbase[256];
  char strbase[256];
  char ltest[256];
  char strtest[256];
    int nbase = sscanf(lbase, "%255[^\r\n]", strbase);
    int ntest = sscanf(ltest, "%255[^\r\n]", strtest);
    assert( nbase != 255 && ntest != 255 );
Fixed code: 	/* all those OPJ_UINT32 are casted to OPJ_INT32, let's do some sanity check */
	if ((l_tile->x0 < 0) || (l_tile->x1 <= l_tile->x0)) {
		opj_event_msg(manager, EVT_ERROR, "Tile X coordinates are not supported\n");
		return OPJ_FALSE;
	}
	/* all those OPJ_UINT32 are casted to OPJ_INT32, let's do some sanity check */
	if ((l_tile->y0 < 0) || (l_tile->y1 <= l_tile->y0)) {
		opj_event_msg(manager, EVT_ERROR, "Tile Y coordinates are not supported\n");
		return OPJ_FALSE;
	}
	
  char lbase[512];
  char strbase[512];
  char ltest[512];
  char strtest[512];
    int nbase = sscanf(lbase, "%511[^\r\n]", strbase);
    int ntest = sscanf(ltest, "%511[^\r\n]", strtest);
    assert( nbase != 511 && ntest != 511 );
# issue 823 (yes, not a typo, test image is issue822)
!opj_decompress -i @INPUT_NR_PATH@/issue822.jp2 -o @TEMP_PATH@/issue822.png