From: Matthieu Darbois <mayeut@users.noreply.github.com>
Date: Thu, 28 Apr 2016 11:16:43 +0000 (+0200)
Subject: Fix unsigned int overflow reported by UBSan (#761)
X-Git-Tag: v2.1.1~18^2~21
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=29313eb5f1b2c01c7493087fa2d8f1a20495a34e;p=openjpeg

Fix unsigned int overflow reported by UBSan (#761)

* Fix unsigned int overflow reported by UBSan

Please add -DOPJ_UBSAN_BUILD to CFLAGS when building with
-fsanitize=undefined,unsigned-integer-overflow

It seems clang/gcc do not allow to disable checking for block of code
other than function or file.
---

diff --git a/src/lib/openjp2/bio.c b/src/lib/openjp2/bio.c
index e4edb372..269769b4 100644
--- a/src/lib/openjp2/bio.c
+++ b/src/lib/openjp2/bio.c
@@ -151,19 +151,31 @@ void opj_bio_init_dec(opj_bio_t *bio, OPJ_BYTE *bp, OPJ_UINT32 len) {
 	bio->ct = 0;
 }
 
+OPJ_NOSANITIZE("unsigned-integer-overflow")
 void opj_bio_write(opj_bio_t *bio, OPJ_UINT32 v, OPJ_UINT32 n) {
 	OPJ_UINT32 i;
-	for (i = n - 1; i < n; i--) {
+	
+	assert((n > 0U) && (n <= 32U));
+	for (i = n - 1; i < n; i--) { /* overflow used for end-loop condition */
 		opj_bio_putbit(bio, (v >> i) & 1);
 	}
 }
 
+OPJ_NOSANITIZE("unsigned-integer-overflow")
 OPJ_UINT32 opj_bio_read(opj_bio_t *bio, OPJ_UINT32 n) {
 	OPJ_UINT32 i;
-    OPJ_UINT32 v;
-	v = 0;
-	for (i = n - 1; i < n; i--) {
-		v += opj_bio_getbit(bio) << i;
+	OPJ_UINT32 v;
+	
+	assert((n > 0U) /* && (n <= 32U)*/);
+#ifdef OPJ_UBSAN_BUILD
+	/* This assert fails for some corrupted images which are gracefully rejected */
+	/* Add this assert only for ubsan build. */
+	/* This is the condition for overflow not to occur below which is needed because of OPJ_NOSANITIZE */
+	assert(n <= 32U);
+#endif
+	v = 0U;
+	for (i = n - 1; i < n; i--) { /* overflow used for end-loop condition */
+		v |= opj_bio_getbit(bio) << i; /* can't overflow, opj_bio_getbit returns 0 or 1 */
 	}
 	return v;
 }
diff --git a/src/lib/openjp2/j2k.c b/src/lib/openjp2/j2k.c
index be8e9442..525f629a 100644
--- a/src/lib/openjp2/j2k.c
+++ b/src/lib/openjp2/j2k.c
@@ -2063,17 +2063,17 @@ static OPJ_BOOL opj_j2k_read_siz(opj_j2k_t *p_j2k,
         /* testcase 4035.pdf.SIGSEGV.d8b.3375 */
         /* testcase issue427-null-image-size.jp2 */
         if ((l_image->x0 >= l_image->x1) || (l_image->y0 >= l_image->y1)) {
-                opj_event_msg(p_manager, EVT_ERROR, "Error with SIZ marker: negative or zero image size (%d x %d)\n", l_image->x1 - l_image->x0, l_image->y1 - l_image->y0);
+                opj_event_msg(p_manager, EVT_ERROR, "Error with SIZ marker: negative or zero image size (%" PRId64 " x %" PRId64 ")\n", (OPJ_INT64)l_image->x1 - l_image->x0, (OPJ_INT64)l_image->y1 - l_image->y0);
                 return OPJ_FALSE;
         }
         /* testcase 2539.pdf.SIGFPE.706.1712 (also 3622.pdf.SIGFPE.706.2916 and 4008.pdf.SIGFPE.706.3345 and maybe more) */
-        if (!(l_cp->tdx * l_cp->tdy)) {
+        if ((l_cp->tdx == 0U) || (l_cp->tdy == 0U)) {
                 opj_event_msg(p_manager, EVT_ERROR, "Error with SIZ marker: invalid tile size (tdx: %d, tdy: %d)\n", l_cp->tdx, l_cp->tdy);
                 return OPJ_FALSE;
         }
 
         /* testcase 1610.pdf.SIGSEGV.59c.681 */
-        if (((OPJ_UINT64)l_image->x1) * ((OPJ_UINT64)l_image->y1) != (l_image->x1 * l_image->y1)) {
+        if ((0xFFFFFFFFU / l_image->x1) < l_image->y1) {
                 opj_event_msg(p_manager, EVT_ERROR, "Prevent buffer overflow (x1: %d, y1: %d)\n", l_image->x1, l_image->y1);
                 return OPJ_FALSE;
         }
diff --git a/src/lib/openjp2/opj_includes.h b/src/lib/openjp2/opj_includes.h
index 5add0918..58a5a9a9 100644
--- a/src/lib/openjp2/opj_includes.h
+++ b/src/lib/openjp2/opj_includes.h
@@ -112,6 +112,14 @@
 	#endif
 #endif
 
+#ifdef __has_attribute
+	#if __has_attribute(no_sanitize)
+		#define OPJ_NOSANITIZE(kind) __attribute__((no_sanitize(kind)))
+	#endif
+#endif
+#ifndef OPJ_NOSANITIZE
+	#define OPJ_NOSANITIZE(kind)
+#endif
 
 
 /* MSVC before 2013 and Borland C do not have lrintf */
diff --git a/src/lib/openjp2/pi.c b/src/lib/openjp2/pi.c
index bfee10a2..cffad668 100644
--- a/src/lib/openjp2/pi.c
+++ b/src/lib/openjp2/pi.c
@@ -747,10 +747,12 @@ static void opj_get_all_encoding_parameters(   const opj_image_t *p_image,
 		}
 
 		/* use custom size for precincts*/
-		l_level_no = l_tccp->numresolutions - 1;
+		l_level_no = l_tccp->numresolutions;
 		for (resno = 0; resno < l_tccp->numresolutions; ++resno) {
 			OPJ_UINT32 l_dx, l_dy;
 
+			--l_level_no;
+			
 			/* precinct width and height*/
 			l_pdx = l_tccp->prcw[resno];
 			l_pdy = l_tccp->prch[resno];
@@ -782,7 +784,6 @@ static void opj_get_all_encoding_parameters(   const opj_image_t *p_image,
 				*p_max_prec = l_product;
 			}
 
-			--l_level_no;
 		}
 		++l_tccp;
 		++l_img_comp;