Vulnerability: 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.
Vulnerability ID: CVE-2016-10250
CWE: CWE-476
Severity score: 2.9
Project: https://github.com/mdadams/jasper
Published: 2017-03-15
File: src/libjasper/jp2/jp2_cod.c
Commit message: Fixed another problem with incorrect cleanup of JP2 box data upon error.
Commit author: Michael Adams

Vulnerable version (before fix):
 jp2_box_t *jp2_box_get(jas_stream_t *in) 	if (!(box = jas_malloc(sizeof(jp2_box_t)))) { 		goto error; 	} 	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", jp2_box_t *jp2_box_get(jas_stream_t *in) 			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);  		if (box->ops->getdata) { 			if ((*box->ops->getdata)(box, tmpstream)) { 				jas_eprintf("cannot parse box data\n"); void jp2_box_dump(jp2_box_t *box, FILE *out) 	assert(boxinfo);  	fprintf(out, "JP2 box: "); 	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); 	}

Fixed version (after fix):
 jp2_box_t *jp2_box_get(jas_stream_t *in) 	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->len = len; 	JAS_DBGLOG(10, ( 	  "preliminary processing of JP2 box: type=%c%s%c (0x%08x); length=%d\n", jp2_box_t *jp2_box_get(jas_stream_t *in) 			goto error; 		} 		if (jas_stream_copy(tmpstream, in, box->datalen)) { 			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"); void jp2_box_dump(jp2_box_t *box, FILE *out) 	assert(boxinfo);  	fprintf(out, "JP2 box: "); 	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); 	}

Patch:
patch: @@ -251,13 +251,16 @@ jp2_box_t *jp2_box_get(jas_stream_t *in)
 	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",
@@ -287,14 +290,15 @@ jp2_box_t *jp2_box_get(jas_stream_t *in)
 			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");
@@ -327,8 +331,8 @@ void jp2_box_dump(jp2_box_t *box, FILE *out)
 	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);
 	}
path: src/libjasper/jp2/jp2_cod.c