CVE-2014-2270: Improper Restriction of Operations within the Bounds of a Memory Buffer (CWE-119)
value: softmagic.c in file before 5.17 and libmagic allows context-dependent attackers to cause a denial of service (out-of-bounds memory access and crash) via crafted offsets in the softmagic of a PE executable.
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: 2014-03-14T15:55Z
Repository: https://github.com/file/file
Language: C
Files changed: src/softmagic.c
Commit date: 2014-01-08 22:22:54 +0000
Commit message: PR/313: Aaron Reffett: Check properly for exceeding the offset.

Diff: diff --git a/src/softmagic.c b/src/softmagic.c
index ec713649..ac9c18bd 100644
--- a/src/softmagic.c
+++ b/src/softmagic.c
@@ -30,11 +30,11 @@
  */
 
 #include "file.h"
 
 #ifndef	lint
-FILE_RCSID("@(#)$File: softmagic.c,v 1.170 2014/01/06 02:25:32 christos Exp $")
+FILE_RCSID("@(#)$File: softmagic.c,v 1.171 2014/01/08 22:02:06 christos Exp $")
 #endif	/* lint */
 
 #include "magic.h"
 #ifdef HAVE_FMTCHECK
 #include <stdio.h>
@@ -69,10 +69,11 @@ private int handle_annotation(struct magic_set *, struct magic *);
 private void cvt_8(union VALUETYPE *, const struct magic *);
 private void cvt_16(union VALUETYPE *, const struct magic *);
 private void cvt_32(union VALUETYPE *, const struct magic *);
 private void cvt_64(union VALUETYPE *, const struct magic *);
 
+#define OFFSET_OOB(n, o, i)	((n) < (o) || (i) >= ((n) - (o)))
 /*
  * softmagic - lookup one file in parsed, in-memory copy of database
  * Passed the name and FILE * of one file to be typed.
  */
 /*ARGSUSED1*/		/* nbytes passed for regularity, maybe need later */
@@ -1221,11 +1222,11 @@ mget(struct magic_set *ms, const unsigned char *s, struct magic *m,
 			if ((ms->flags & MAGIC_DEBUG) != 0)
 				fprintf(stderr, "indirect offs=%u\n", off);
 		}
 		switch (in_type = cvt_flip(m->in_type, flip)) {
 		case FILE_BYTE:
-			if (nbytes < offset || nbytes < (offset + 1))
+			if (OFFSET_OOB(nbytes, offset, 1))
 				return 0;
 			if (off) {
 				switch (m->in_op & FILE_OPS_MASK) {
 				case FILE_OPAND:
 					offset = p->b & off;
@@ -1256,11 +1257,11 @@ mget(struct magic_set *ms, const unsigned char *s, struct magic *m,
 				offset = p->b;
 			if (m->in_op & FILE_OPINVERSE)
 				offset = ~offset;
 			break;
 		case FILE_BESHORT:
-			if (nbytes < offset || nbytes < (offset + 2))
+			if (OFFSET_OOB(nbytes, offset, 2))
 				return 0;
 			if (off) {
 				switch (m->in_op & FILE_OPS_MASK) {
 				case FILE_OPAND:
 					offset = (short)((p->hs[0]<<8)|
@@ -1308,11 +1309,11 @@ mget(struct magic_set *ms, const unsigned char *s, struct magic *m,
 						 (p->hs[1]));
 			if (m->in_op & FILE_OPINVERSE)
 				offset = ~offset;
 			break;
 		case FILE_LESHORT:
-			if (nbytes < offset || nbytes < (offset + 2))
+			if (OFFSET_OOB(nbytes, offset, 2))
 				return 0;
 			if (off) {
 				switch (m->in_op & FILE_OPS_MASK) {
 				case FILE_OPAND:
 					offset = (short)((p->hs[1]<<8)|
@@ -1360,11 +1361,11 @@ mget(struct magic_set *ms, const unsigned char *s, struct magic *m,
 						 (p->hs[0]));
 			if (m->in_op & FILE_OPINVERSE)
 				offset = ~offset;
 			break;
 		case FILE_SHORT:
-			if (nbytes < offset || nbytes < (offset + 2))
+			if (OFFSET_OOB(nbytes, offset, 2))
 				return 0;
 			if (off) {
 				switch (m->in_op & FILE_OPS_MASK) {
 				case FILE_OPAND:
 					offset = p->h & off;
@@ -1397,11 +1398,11 @@ mget(struct magic_set *ms, const unsigned char *s, struct magic *m,
 			if (m->in_op & FILE_OPINVERSE)
 				offset = ~offset;
 			break;
 		case FILE_BELONG:
 		case FILE_BEID3:
-			if (nbytes < offset || nbytes < (offset + 4))
+			if (OFFSET_OOB(nbytes, offset, 4))
 				return 0;
 			if (off) {
 				switch (m->in_op & FILE_OPS_MASK) {
 				case FILE_OPAND:
 					offset = (int32_t)((p->hl[0]<<24)|
@@ -1468,11 +1469,11 @@ mget(struct magic_set *ms, const unsigned char *s, struct magic *m,
 			if (m->in_op & FILE_OPINVERSE)
 				offset = ~offset;
 			break;
 		case FILE_LELONG:
 		case FILE_LEID3:
-			if (nbytes < offset || nbytes < (offset + 4))
+			if (OFFSET_OOB(nbytes, offset, 4))
 				return 0;
 			if (off) {
 				switch (m->in_op & FILE_OPS_MASK) {
 				case FILE_OPAND:
 					offset = (int32_t)((p->hl[3]<<24)|
@@ -1538,11 +1539,11 @@ mget(struct magic_set *ms, const unsigned char *s, struct magic *m,
 						 (p->hl[0]));
 			if (m->in_op & FILE_OPINVERSE)
 				offset = ~offset;
 			break;
 		case FILE_MELONG:
-			if (nbytes < offset || nbytes < (offset + 4))
+			if (OFFSET_OOB(nbytes, offset, 4))
 				return 0;
 			if (off) {
 				switch (m->in_op & FILE_OPS_MASK) {
 				case FILE_OPAND:
 					offset = (int32_t)((p->hl[1]<<24)|
@@ -1608,11 +1609,11 @@ mget(struct magic_set *ms, const unsigned char *s, struct magic *m,
 						 (p->hl[2]));
 			if (m->in_op & FILE_OPINVERSE)
 				offset = ~offset;
 			break;
 		case FILE_LONG:
-			if (nbytes < offset || nbytes < (offset + 4))
+			if (OFFSET_OOB(nbytes, offset, 4))
 				return 0;
 			if (off) {
 				switch (m->in_op & FILE_OPS_MASK) {
 				case FILE_OPAND:
 					offset = p->l & off;
@@ -1685,18 +1686,18 @@ mget(struct magic_set *ms, const unsigned char *s, struct magic *m,
 	}
 
 	/* Verify we have enough data to match magic type */
 	switch (m->type) {
 	case FILE_BYTE:
-		if (nbytes < (offset + 1)) /* should alway be true */
+		if (OFFSET_OOB(nbytes, offset, 1))
 			return 0;
 		break;
 
 	case FILE_SHORT:
 	case FILE_BESHORT:
 	case FILE_LESHORT:
-		if (nbytes < (offset + 2))
+		if (OFFSET_OOB(nbytes, offset, 2))
 			return 0;
 		break;
 
 	case FILE_LONG:
 	case FILE_BELONG:
@@ -1711,35 +1712,35 @@ mget(struct magic_set *ms, const unsigned char *s, struct magic *m,
 	case FILE_LELDATE:
 	case FILE_MELDATE:
 	case FILE_FLOAT:
 	case FILE_BEFLOAT:
 	case FILE_LEFLOAT:
-		if (nbytes < (offset + 4))
+		if (OFFSET_OOB(nbytes, offset, 4))
 			return 0;
 		break;
 
 	case FILE_DOUBLE:
 	case FILE_BEDOUBLE:
 	case FILE_LEDOUBLE:
-		if (nbytes < (offset + 8))
+		if (OFFSET_OOB(nbytes, offset, 8))
 			return 0;
 		break;
 
 	case FILE_STRING:
 	case FILE_PSTRING:
 	case FILE_SEARCH:
-		if (nbytes < (offset + m->vallen))
+		if (OFFSET_OOB(nbytes, offset, m->vallen))
 			return 0;
 		break;
 
 	case FILE_REGEX:
-		if (nbytes < offset)
+		if (OFFSET_OOB(nbytes, offset, 0))
 			return 0;
 		break;
 
 	case FILE_INDIRECT:
-		if (nbytes < offset)
+		if (OFFSET_OOB(nbytes, offset, 0))
 			return 0;
 		sbuf = ms->o.buf;
 		soffset = ms->offset;
 		ms->o.buf = NULL;
 		ms->offset = 0;
@@ -1759,11 +1760,11 @@ mget(struct magic_set *ms, const unsigned char *s, struct magic *m,
 			free(rbuf);
 		}
 		return rv;
 
 	case FILE_USE:
-		if (nbytes < offset)
+		if (OFFSET_OOB(nbytes, offset, 0))
 			return 0;
 		sbuf = m->value.s;
 		if (*sbuf == '^') {
 			sbuf++;
 			flip = !flip;

Vulnerable code: FILE_RCSID("@(#)$File: softmagic.c,v 1.170 2014/01/06 02:25:32 christos Exp $")
			if (nbytes < offset || nbytes < (offset + 1))
			if (nbytes < offset || nbytes < (offset + 2))
			if (nbytes < offset || nbytes < (offset + 2))
			if (nbytes < offset || nbytes < (offset + 2))
			if (nbytes < offset || nbytes < (offset + 4))
			if (nbytes < offset || nbytes < (offset + 4))
			if (nbytes < offset || nbytes < (offset + 4))
			if (nbytes < offset || nbytes < (offset + 4))
		if (nbytes < (offset + 1)) /* should alway be true */
		if (nbytes < (offset + 2))
		if (nbytes < (offset + 4))
		if (nbytes < (offset + 8))
		if (nbytes < (offset + m->vallen))
		if (nbytes < offset)
		if (nbytes < offset)
		if (nbytes < offset)
Fixed code: FILE_RCSID("@(#)$File: softmagic.c,v 1.171 2014/01/08 22:02:06 christos Exp $")
#define OFFSET_OOB(n, o, i)	((n) < (o) || (i) >= ((n) - (o)))
			if (OFFSET_OOB(nbytes, offset, 1))
			if (OFFSET_OOB(nbytes, offset, 2))
			if (OFFSET_OOB(nbytes, offset, 2))
			if (OFFSET_OOB(nbytes, offset, 2))
			if (OFFSET_OOB(nbytes, offset, 4))
			if (OFFSET_OOB(nbytes, offset, 4))
			if (OFFSET_OOB(nbytes, offset, 4))
			if (OFFSET_OOB(nbytes, offset, 4))
		if (OFFSET_OOB(nbytes, offset, 1))
		if (OFFSET_OOB(nbytes, offset, 2))
		if (OFFSET_OOB(nbytes, offset, 4))
		if (OFFSET_OOB(nbytes, offset, 8))
		if (OFFSET_OOB(nbytes, offset, m->vallen))
		if (OFFSET_OOB(nbytes, offset, 0))
		if (OFFSET_OOB(nbytes, offset, 0))
		if (OFFSET_OOB(nbytes, offset, 0))