CVE-2012-1571: Improper Restriction of Operations within the Bounds of a Memory Buffer (CWE-119)
value: file before 5.11 and libmagic allow remote attackers to cause a denial of service (crash) via a crafted Composite Document File (CDF) file that triggers (1) an out-of-bounds read or (2) an invalid pointer dereference.
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.
Published: 2012-07-17T21:55Z
Repository: https://github.com/glensc/file
Language: C
Files changed: src/cdf.c
Commit date: 2012-02-20 22:35:29 +0000
Commit message: Fix bounds checks again.

Diff: diff --git a/src/cdf.c b/src/cdf.c
index 360c95c..02aed54 100644
--- a/src/cdf.c
+++ b/src/cdf.c
@@ -33,11 +33,11 @@
  */
 
 #include "file.h"
 
 #ifndef lint
-FILE_RCSID("@(#)$File: cdf.c,v 1.48 2012/02/17 05:27:45 christos Exp $")
+FILE_RCSID("@(#)$File: cdf.c,v 1.49 2012/02/20 20:04:37 christos Exp $")
 #endif
 
 #include <assert.h>
 #ifdef CDF_DEBUG
 #include <err.h>
@@ -73,10 +73,11 @@ static union {
 #define CDF_TOLE8(x)	((uint64_t)(NEED_SWAP ? _cdf_tole8(x) : (uint64_t)(x)))
 #define CDF_TOLE4(x)	((uint32_t)(NEED_SWAP ? _cdf_tole4(x) : (uint32_t)(x)))
 #define CDF_TOLE2(x)	((uint16_t)(NEED_SWAP ? _cdf_tole2(x) : (uint16_t)(x)))
 #define CDF_GETUINT32(x, y)	cdf_getuint32(x, y)
 
+
 /*
  * swap a short
  */
 static uint16_t
 _cdf_tole2(uint16_t sv)
@@ -349,17 +350,17 @@ cdf_read_sector(const cdf_info_t *info, void *buf, size_t offs, size_t len,
 
 ssize_t
 cdf_read_short_sector(const cdf_stream_t *sst, void *buf, size_t offs,
     size_t len, const cdf_header_t *h, cdf_secid_t id)
 {
-	size_t ss = CDF_SEC_SIZE(h);
+	size_t ss = CDF_SHORT_SEC_SIZE(h);
 	size_t pos = CDF_SHORT_SEC_POS(h, id);
 	assert(ss == len);
-	if (pos > ss * sst->sst_len) {
+	if (pos > CDF_SEC_SIZE(h) * sst->sst_len) {
 		DPRINTF(("Out of bounds read %" SIZE_T_FORMAT "u > %"
 		    SIZE_T_FORMAT "u\n",
-		    pos, ss * sst->sst_len));
+		    pos, CDF_SEC_SIZE(h) * sst->sst_len));
 		return -1;
 	}
 	(void)memcpy(((char *)buf) + offs,
 	    ((const char *)sst->sst_tab) + pos, len);
 	return len;
@@ -527,11 +528,11 @@ out:
 int
 cdf_read_short_sector_chain(const cdf_header_t *h,
     const cdf_sat_t *ssat, const cdf_stream_t *sst,
     cdf_secid_t sid, size_t len, cdf_stream_t *scn)
 {
-	size_t ss = CDF_SEC_SIZE(h), i, j;
+	size_t ss = CDF_SHORT_SEC_SIZE(h), i, j;
 	scn->sst_len = cdf_count_chain(ssat, sid, CDF_SEC_SIZE(h));
 	scn->sst_dirlen = len;
 
 	if (sst->sst_tab == NULL || scn->sst_len == (size_t)-1)
 		return -1;
@@ -796,22 +797,22 @@ cdf_read_property_info(const cdf_stream_t *sst, const cdf_header_t *h,
 	e = CAST(const uint8_t *, (const void *)
 	    (((const char *)(const void *)shp) + sh.sh_len));
 	if (cdf_check_stream_offset(sst, h, e, 0, __LINE__) == -1)
 		goto out;
 	for (i = 0; i < sh.sh_properties; i++) {
+		size_t ofs = CDF_GETUINT32(p, (i << 1) + 1);
 		q = (const uint8_t *)(const void *)
-		    ((const char *)(const void *)p +
-		    CDF_GETUINT32(p, (i << 1) + 1)) - 2 * sizeof(uint32_t);
+		    ((const char *)(const void *)p + ofs
+		    - 2 * sizeof(uint32_t));
 		if (q > e) {
 			DPRINTF(("Ran of the end %p > %p\n", q, e));
 			goto out;
 		}
 		inp[i].pi_id = CDF_GETUINT32(p, i << 1);
 		inp[i].pi_type = CDF_GETUINT32(q, 0);
-		DPRINTF(("%" SIZE_T_FORMAT "u) id=%x type=%x offs=0x%tx,0x%x\n", i,
-		    inp[i].pi_id, inp[i].pi_type, q - p,
-		    CDF_GETUINT32(p, (i << 1) + 1)));
+		DPRINTF(("%" SIZE_T_FORMAT "u) id=%x type=%x offs=0x%tx,0x%x\n",
+		    i, inp[i].pi_id, inp[i].pi_type, q - p, offs));
 		if (inp[i].pi_type & CDF_VECTOR) {
 			nelements = CDF_GETUINT32(q, 1);
 			o = 2;
 		} else {
 			nelements = 1;

Vulnerable code: FILE_RCSID("@(#)$File: cdf.c,v 1.48 2012/02/17 05:27:45 christos Exp $")
	size_t ss = CDF_SEC_SIZE(h);
	if (pos > ss * sst->sst_len) {
		    pos, ss * sst->sst_len));
	size_t ss = CDF_SEC_SIZE(h), i, j;
		    ((const char *)(const void *)p +
		    CDF_GETUINT32(p, (i << 1) + 1)) - 2 * sizeof(uint32_t);
		DPRINTF(("%" SIZE_T_FORMAT "u) id=%x type=%x offs=0x%tx,0x%x\n", i,
		    inp[i].pi_id, inp[i].pi_type, q - p,
		    CDF_GETUINT32(p, (i << 1) + 1)));
Fixed code: FILE_RCSID("@(#)$File: cdf.c,v 1.49 2012/02/20 20:04:37 christos Exp $")

	size_t ss = CDF_SHORT_SEC_SIZE(h);
	if (pos > CDF_SEC_SIZE(h) * sst->sst_len) {
		    pos, CDF_SEC_SIZE(h) * sst->sst_len));
	size_t ss = CDF_SHORT_SEC_SIZE(h), i, j;
		size_t ofs = CDF_GETUINT32(p, (i << 1) + 1);
		    ((const char *)(const void *)p + ofs
		    - 2 * sizeof(uint32_t));
		DPRINTF(("%" SIZE_T_FORMAT "u) id=%x type=%x offs=0x%tx,0x%x\n",
		    i, inp[i].pi_id, inp[i].pi_type, q - p, offs));