CVE-2016-10250: NULL Pointer Dereference (CWE-476)
value: The jp2_colr_destroy function in jp2_cod.c in JasPer before 1.900.13 allows remote attackers to cause a denial of service (NULL pointer dereference) by leveraging incorrect cleanup of JP2 box data on error. NOTE: this vulnerability exists because of an incomplete fix for CVE-2016-8887.
CWE description: The product dereferences a pointer that it expects to be valid but is NULL.
CVSS v3 base score: 7.5
Published: 2017-03-15T14:59Z
Repository: https://github.com/mdadams/jasper
Language: C
Files changed: src/libjasper/jp2/jp2_cod.c
Commit date: 2016-10-23 03:52:29 -0700
Commit message: Fixed another problem with incorrect cleanup of JP2 box data upon error.

Diff: diff --git a/src/libjasper/jp2/jp2_cod.c b/src/libjasper/jp2/jp2_cod.c
index 2f14234..7f3608a 100644
--- a/src/libjasper/jp2/jp2_cod.c
+++ b/src/libjasper/jp2/jp2_cod.c
@@ -249,17 +249,20 @@ jp2_box_t *jp2_box_get(jas_stream_t *in)
 	tmpstream = 0;
 
 	if (!(box = jas_malloc(sizeof(jp2_box_t)))) {
 		goto error;
 	}
+
+	// Mark the box data as never having been constructed
+	// so that we will not errantly attempt to destroy it later.
 	box->ops = &jp2_boxinfo_unk.ops;
+
 	if (jp2_getuint32(in, &len) || jp2_getuint32(in, &box->type)) {
 		goto error;
 	}
 	boxinfo = jp2_boxinfolookup(box->type);
 	box->info = boxinfo;
-	box->ops = &boxinfo->ops;
 	box->len = len;
 	JAS_DBGLOG(10, (
 	  "preliminary processing of JP2 box: type=%c%s%c (0x%08x); length=%d\n",
 	  '"', boxinfo->name, '"', box->type, box->len
 	  ));
@@ -285,18 +288,19 @@ jp2_box_t *jp2_box_get(jas_stream_t *in)
 	if (dataflag) {
 		if (!(tmpstream = jas_stream_memopen(0, 0))) {
 			goto error;
 		}
 		if (jas_stream_copy(tmpstream, in, box->datalen)) {
-			// Mark the box data as never having been constructed
-			// so that we will not errantly attempt to destroy it later.
-			box->ops = &jp2_boxinfo_unk.ops;
 			jas_eprintf("cannot copy box data\n");
 			goto error;
 		}
 		jas_stream_rewind(tmpstream);
 
+		// From here onwards, the box data will need to be destroyed.
+		// So, initialize the box operations.
+		box->ops = &boxinfo->ops;
+
 		if (box->ops->getdata) {
 			if ((*box->ops->getdata)(box, tmpstream)) {
 				jas_eprintf("cannot parse box data\n");
 				goto error;
 			}
@@ -325,12 +329,12 @@ void jp2_box_dump(jp2_box_t *box, FILE *out)
 	jp2_boxinfo_t *boxinfo;
 	boxinfo = jp2_boxinfolookup(box->type);
 	assert(boxinfo);
 
 	fprintf(out, "JP2 box: ");
-	fprintf(out, "type=%c%s%c (0x%08"PRIxFAST32"); length=%"PRIuFAST32"\n", '"', boxinfo->name,
-	  '"', box->type, box->len);
+	fprintf(out, "type=%c%s%c (0x%08"PRIxFAST32"); length=%"PRIuFAST32"\n", '"',
+	  boxinfo->name, '"', box->type, box->len);
 	if (box->ops->dumpdata) {
 		(*box->ops->dumpdata)(box, out);
 	}
 }
 

Vulnerable code: 	box->ops = &boxinfo->ops;
			// Mark the box data as never having been constructed
			// so that we will not errantly attempt to destroy it later.
			box->ops = &jp2_boxinfo_unk.ops;
	fprintf(out, "type=%c%s%c (0x%08"PRIxFAST32"); length=%"PRIuFAST32"\n", '"', boxinfo->name,
	  '"', box->type, box->len);
Fixed code: 
	// Mark the box data as never having been constructed
	// so that we will not errantly attempt to destroy it later.

		// From here onwards, the box data will need to be destroyed.
		// So, initialize the box operations.
		box->ops = &boxinfo->ops;

	fprintf(out, "type=%c%s%c (0x%08"PRIxFAST32"); length=%"PRIuFAST32"\n", '"',
	  boxinfo->name, '"', box->type, box->len);