CVE-2019-16346: Incorrect Calculation (CWE-682)
value: ngiflib 0.4 has a heap-based buffer overflow in WritePixel() in ngiflib.c when called from DecodeGifImg, because deinterlacing for small pictures is mishandled.
CWE description: The product performs a calculation that generates incorrect or unintended results that are later used in security-critical decisions or resource management.
CVSS v3 base score: 8.8
Published: 2019-09-16T13:15Z
Repository: https://github.com/miniupnp/ngiflib
Language: C
Files changed: ngiflib.c
Commit date: 2019-04-16 02:07:58 +0200
Commit message: fix deinterlacing for small pictures

fixes #12

Diff: diff --git a/invalid_gif/issue_12.gif b/invalid_gif/issue_12.gif
new file mode 100644
index 0000000..48e19c3
Binary files /dev/null and b/invalid_gif/issue_12.gif differ
diff --git a/ngiflib.c b/ngiflib.c
index 05321f1..1d29313 100644
--- a/ngiflib.c
+++ b/ngiflib.c
@@ -4,11 +4,11 @@
 
 #include "ngiflib.h"
 
 /* decodeur GIF en C portable (pas de pb big/little endian)
  * Thomas BERNARD. janvier 2004.
- * (c) 2004-2017 Thomas Bernard. All rights reserved
+ * (c) 2004-2019 Thomas Bernard. All rights reserved
  */
 
 /* Fonction de debug */
 #ifdef DEBUG
 void fprintf_ngiflib_img(FILE * f, struct ngiflib_img * i) {
@@ -133,33 +133,35 @@ static void WritePixel(struct ngiflib_img * i, struct ngiflib_decode_context * c
 		case 0:
 			context->curY++;
 			break;
 		case 1:	/* 1st pass : every eighth row starting from 0 */
 			context->curY += 8;
-			if(context->curY >= p->height) {
-				context->pass++;
-				context->curY = i->posY + 4;
-			}
 			break;
 		case 2:	/* 2nd pass : every eighth row starting from 4 */
 			context->curY += 8;
-			if(context->curY >= p->height) {
-				context->pass++;
-				context->curY = i->posY + 2;
-			}
 			break;
 		case 3:	/* 3rd pass : every fourth row starting from 2 */
 			context->curY += 4;
-			if(context->curY >= p->height) {
-				context->pass++;
-				context->curY = i->posY + 1;
-			}
 			break;
 		case 4:	/* 4th pass : every odd row */
 			context->curY += 2;
 			break;
 		}
+		while(context->pass > 0 && context->pass < 4 &&
+		      context->curY >= p->height) {
+			switch(++context->pass) {
+			case 2:	/* 2nd pass : every eighth row starting from 4 */
+				context->curY = i->posY + 4;
+				break;
+			case 3:	/* 3rd pass : every fourth row starting from 2 */
+				context->curY = i->posY + 2;
+				break;
+			case 4:	/* 4th pass : every odd row */
+				context->curY = i->posY + 1;
+				break;
+			}
+		}
 #ifndef NGIFLIB_INDEXED_ONLY
 		if(p->mode & NGIFLIB_MODE_INDEXED) {
 #endif /* NGIFLIB_INDEXED_ONLY */
 			#ifdef NGIFLIB_ENABLE_CALLBACKS
 			context->line_p.p8 = p->frbuff.p8 + (u32)context->curY*p->width;
@@ -247,33 +249,35 @@ static void WritePixels(struct ngiflib_img * i, struct ngiflib_decode_context *
 			case 0:
 				context->curY++;
 				break;
 			case 1:	/* 1st pass : every eighth row starting from 0 */
 				context->curY += 8;
-				if(context->curY >= p->height) {
-					context->pass++;
-					context->curY = i->posY + 4;
-				}
 				break;
 			case 2:	/* 2nd pass : every eighth row starting from 4 */
 				context->curY += 8;
-				if(context->curY >= p->height) {
-					context->pass++;
-					context->curY = i->posY + 2;
-				}
 				break;
 			case 3:	/* 3rd pass : every fourth row starting from 2 */
 				context->curY += 4;
-				if(context->curY >= p->height) {
-					context->pass++;
-					context->curY = i->posY + 1;
-				}
 				break;
 			case 4:	/* 4th pass : every odd row */
 				context->curY += 2;
 				break;
 			}
+			while(context->pass > 0 && context->pass < 4 &&
+			      context->curY >= p->height) {
+				switch(++context->pass) {
+				case 2:	/* 2nd pass : every eighth row starting from 4 */
+					context->curY = i->posY + 4;
+					break;
+				case 3:	/* 3rd pass : every fourth row starting from 2 */
+					context->curY = i->posY + 2;
+					break;
+				case 4:	/* 4th pass : every odd row */
+					context->curY = i->posY + 1;
+					break;
+				}
+			}
 #ifndef NGIFLIB_INDEXED_ONLY
 			if(p->mode & NGIFLIB_MODE_INDEXED) {
 #endif /* NGIFLIB_INDEXED_ONLY */
 				#ifdef NGIFLIB_ENABLE_CALLBACKS
 				context->line_p.p8 = p->frbuff.p8 + (u32)context->curY*p->width;

Vulnerable code:  * (c) 2004-2017 Thomas Bernard. All rights reserved
			if(context->curY >= p->height) {
				context->pass++;
				context->curY = i->posY + 4;
			}
			if(context->curY >= p->height) {
				context->pass++;
				context->curY = i->posY + 2;
			}
			if(context->curY >= p->height) {
				context->pass++;
				context->curY = i->posY + 1;
			}
				if(context->curY >= p->height) {
					context->pass++;
					context->curY = i->posY + 4;
				}
				if(context->curY >= p->height) {
					context->pass++;
					context->curY = i->posY + 2;
				}
				if(context->curY >= p->height) {
					context->pass++;
					context->curY = i->posY + 1;
				}
Fixed code:  * (c) 2004-2019 Thomas Bernard. All rights reserved
		while(context->pass > 0 && context->pass < 4 &&
		      context->curY >= p->height) {
			switch(++context->pass) {
			case 2:	/* 2nd pass : every eighth row starting from 4 */
				context->curY = i->posY + 4;
				break;
			case 3:	/* 3rd pass : every fourth row starting from 2 */
				context->curY = i->posY + 2;
				break;
			case 4:	/* 4th pass : every odd row */
				context->curY = i->posY + 1;
				break;
			}
		}
			while(context->pass > 0 && context->pass < 4 &&
			      context->curY >= p->height) {
				switch(++context->pass) {
				case 2:	/* 2nd pass : every eighth row starting from 4 */
					context->curY = i->posY + 4;
					break;
				case 3:	/* 3rd pass : every fourth row starting from 2 */
					context->curY = i->posY + 2;
					break;
				case 4:	/* 4th pass : every odd row */
					context->curY = i->posY + 1;
					break;
				}
			}