From: Christos Zoulas <christos@zoulas.com>
Date: Mon, 21 Mar 2016 17:41:14 +0000 (+0000)
Subject: Add bounds checks to DER code offset computations.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=8c42a66ff032ecce25a2bf3844f04e5be3151a13;p=file

Add bounds checks to DER code offset computations.
---

diff --git a/ChangeLog b/ChangeLog
index 24c06c8a..75730b1d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,6 @@
-2016-03-21  11:56  Christos Zoulas <christos@zoulas.com>
+2016-03-21  13:40  Christos Zoulas <christos@zoulas.com>
 
+	* Add bounds checks for DER code (discovered by Thomas Jarosch)
 	* Change indirect recursion limit to indirect use count and
 	  bump from 15 to 50 to prevent abuse.
 
diff --git a/src/der.c b/src/der.c
index b7c341bf..c55bcd58 100644
--- a/src/der.c
+++ b/src/der.c
@@ -35,7 +35,7 @@
 #include "file.h"
 
 #ifndef lint
-FILE_RCSID("@(#)$File: der.c,v 1.2 2016/01/19 15:09:28 christos Exp $")
+FILE_RCSID("@(#)$File: der.c,v 1.3 2016/03/21 17:41:14 christos Exp $")
 #endif
 #endif
 
@@ -55,6 +55,8 @@ FILE_RCSID("@(#)$File: der.c,v 1.2 2016/01/19 15:09:28 christos Exp $")
 #include "der.h"
 #endif
 
+#define DER_BAD	((uint32_t)-1)
+
 #define DER_CLASS_UNIVERSAL	0
 #define	DER_CLASS_APPLICATION	1
 #define	DER_CLASS_CONTEXT	2
@@ -131,14 +133,24 @@ gettype(uint8_t c)
 static uint32_t
 gettag(const uint8_t *c, size_t *p, size_t l)
 {
-	uint32_t tag = c[(*p)++] & 0x1f;
-	l--;
+	uint32_t tag;
+
+	if (*p >= l)
+		return DER_BAD;
+
+	tag = c[(*p)++] & 0x1f;
 
 	if (tag != 0x1f)
 		return tag;
 
-	while (c[*p] >= 0x80 && l-- > 0)
+	if (*p >= l)
+		return DER_BAD;
+
+	while (c[*p] >= 0x80) {
 		tag = tag * 128 + c[(*p)++] - 0x80;
+		if (*p >= l)
+			return DER_BAD;
+	}
 	return tag;
 }
 
@@ -148,15 +160,21 @@ getlength(const uint8_t *c, size_t *p, size_t l)
 	uint8_t digits, i;
 	size_t len;
 
+	if (*p >= l)
+		return DER_BAD;
+
 	digits = c[(*p)++];
-	l--;
+
         if ((digits & 0x80) == 0)
 		return digits;
 
         digits &= 0x7f;
 	len = 0;
 
-	for (i = 0; i < digits && l-- > 0; i++)
+	if (*p + digits >= l)
+		return DER_BAD;
+
+	for (i = 0; i < digits; i++)
 		len = (len << 8) | c[(*p)++];
         return len;
 }
@@ -199,7 +217,8 @@ der_offs(struct magic_set *ms, struct magic *m)
 {
 	const uint8_t *b = CAST(const void *, ms->search.s);
 	size_t offs = 0, len = ms->search.rm_len;
-	(void)gettag(b, &offs, len);
+	if (gettag(b, &offs, len) == DER_BAD)
+		return -1;
 	DPRINTF(("%s1: %d %zu %u\n", __func__, ms->offset, offs, m->offset));
 	uint32_t tlen = getlength(b, &offs, len);
 	DPRINTF(("%s2: %d %zu %u\n", __func__, ms->offset, offs, tlen));
@@ -223,10 +242,17 @@ der_cmp(struct magic_set *ms, struct magic *m)
 	const uint8_t *b = CAST(const void *, ms->search.s);
 	const char *s = m->value.s;
 	size_t offs = 0, len = ms->search.s_len;
-	uint32_t tag = gettag(b, &offs, len);
-	uint32_t tlen = getlength(b, &offs, len);
+	uint32_t tag, tlen;
 	char buf[128];
 
+	tag = gettag(b, &offs, len);
+	if (tag == DER_BAD)
+		return -1;
+
+	tlen = getlength(b, &offs, len);
+	if (tlen == DER_BAD)
+		return -1;
+
 	der_tag(buf, sizeof(buf), tag);
 	if ((ms->flags & MAGIC_DEBUG) != 0)
 		fprintf(stderr, "%s: tag %p got=%s exp=%s\n", __func__, b,
diff --git a/src/softmagic.c b/src/softmagic.c
index e2e672d8..fc388ff7 100644
--- a/src/softmagic.c
+++ b/src/softmagic.c
@@ -32,7 +32,7 @@
 #include "file.h"
 
 #ifndef	lint
-FILE_RCSID("@(#)$File: softmagic.c,v 1.227 2016/03/21 15:56:53 christos Exp $")
+FILE_RCSID("@(#)$File: softmagic.c,v 1.228 2016/03/21 17:41:14 christos Exp $")
 #endif	/* lint */
 
 #include "magic.h"
@@ -51,7 +51,7 @@ private int mget(struct magic_set *, const unsigned char *,
     uint16_t *, int *, int *, int *);
 private int magiccheck(struct magic_set *, struct magic *);
 private int32_t mprint(struct magic_set *, struct magic *);
-private int32_t moffset(struct magic_set *, struct magic *);
+private int moffset(struct magic_set *, struct magic *, int32_t *);
 private void mdebug(uint32_t, const char *, size_t);
 private int mcopy(struct magic_set *, union VALUETYPE *, int, int,
     const unsigned char *, uint32_t, size_t, struct magic *);
@@ -255,7 +255,8 @@ match(struct magic_set *ms, struct magic *magic, uint32_t nmagic,
 		if (print && mprint(ms, m) == -1)
 			return -1;
 
-		ms->c.li[cont_level].off = moffset(ms, m);
+		if (moffset(ms, m, &ms->c.li[cont_level].off) == -1)
+			return -1;
 
 		/* and any continuations that match */
 		if (file_check_mem(ms, ++cont_level) == -1)
@@ -361,7 +362,9 @@ match(struct magic_set *ms, struct magic *magic, uint32_t nmagic,
 				if (print && mprint(ms, m) == -1)
 					return -1;
 
-				ms->c.li[cont_level].off = moffset(ms, m);
+				if (moffset(ms, m, &ms->c.li[cont_level].off)
+				    == -1)
+					return -1;
 
 				if (*m->desc)
 					*need_separator = 1;
@@ -702,36 +705,40 @@ mprint(struct magic_set *ms, struct magic *m)
 	return (int32_t)t;
 }
 
-private int32_t
-moffset(struct magic_set *ms, struct magic *m)
+private int
+moffset(struct magic_set *ms, struct magic *m, int32_t *o)
 {
   	switch (m->type) {
   	case FILE_BYTE:
-		return CAST(int32_t, (ms->offset + sizeof(char)));
+		*o = CAST(int32_t, (ms->offset + sizeof(char)));
+		return 0;
 
   	case FILE_SHORT:
   	case FILE_BESHORT:
   	case FILE_LESHORT:
-		return CAST(int32_t, (ms->offset + sizeof(short)));
+		*o = CAST(int32_t, (ms->offset + sizeof(short)));
+		return 0;
 
   	case FILE_LONG:
   	case FILE_BELONG:
   	case FILE_LELONG:
   	case FILE_MELONG:
-		return CAST(int32_t, (ms->offset + sizeof(int32_t)));
+		*o = CAST(int32_t, (ms->offset + sizeof(int32_t)));
+		return 0;
 
   	case FILE_QUAD:
   	case FILE_BEQUAD:
   	case FILE_LEQUAD:
-		return CAST(int32_t, (ms->offset + sizeof(int64_t)));
+		*o = CAST(int32_t, (ms->offset + sizeof(int64_t)));
+		return 0;
 
   	case FILE_STRING:
   	case FILE_PSTRING:
   	case FILE_BESTRING16:
   	case FILE_LESTRING16:
-		if (m->reln == '=' || m->reln == '!')
-			return ms->offset + m->vallen;
-		else {
+		if (m->reln == '=' || m->reln == '!') {
+			*o = ms->offset + m->vallen;
+		} else {
 			union VALUETYPE *p = &ms->ms_value;
 			uint32_t t;
 
@@ -740,60 +747,79 @@ moffset(struct magic_set *ms, struct magic *m)
 			t = CAST(uint32_t, (ms->offset + strlen(p->s)));
 			if (m->type == FILE_PSTRING)
 				t += (uint32_t)file_pstring_length_size(m);
-			return t;
+			*o = t;
 		}
+		return 0;
 
 	case FILE_DATE:
 	case FILE_BEDATE:
 	case FILE_LEDATE:
 	case FILE_MEDATE:
-		return CAST(int32_t, (ms->offset + sizeof(uint32_t)));
+		*o = CAST(int32_t, (ms->offset + sizeof(uint32_t)));
+		return 0;
 
 	case FILE_LDATE:
 	case FILE_BELDATE:
 	case FILE_LELDATE:
 	case FILE_MELDATE:
-		return CAST(int32_t, (ms->offset + sizeof(uint32_t)));
+		*o = CAST(int32_t, (ms->offset + sizeof(uint32_t)));
+		return 0;
 
 	case FILE_QDATE:
 	case FILE_BEQDATE:
 	case FILE_LEQDATE:
-		return CAST(int32_t, (ms->offset + sizeof(uint64_t)));
+		*o = CAST(int32_t, (ms->offset + sizeof(uint64_t)));
+		return 0;
 
 	case FILE_QLDATE:
 	case FILE_BEQLDATE:
 	case FILE_LEQLDATE:
-		return CAST(int32_t, (ms->offset + sizeof(uint64_t)));
+		*o = CAST(int32_t, (ms->offset + sizeof(uint64_t)));
+		return 0;
 
   	case FILE_FLOAT:
   	case FILE_BEFLOAT:
   	case FILE_LEFLOAT:
-		return CAST(int32_t, (ms->offset + sizeof(float)));
+		*o = CAST(int32_t, (ms->offset + sizeof(float)));
+		return 0;
 
   	case FILE_DOUBLE:
   	case FILE_BEDOUBLE:
   	case FILE_LEDOUBLE:
-		return CAST(int32_t, (ms->offset + sizeof(double)));
+		*o = CAST(int32_t, (ms->offset + sizeof(double)));
+		return 0;
 
 	case FILE_REGEX:
 		if ((m->str_flags & REGEX_OFFSET_START) != 0)
-			return CAST(int32_t, ms->search.offset);
+			*o = CAST(int32_t, ms->search.offset);
 		else
-			return CAST(int32_t, (ms->search.offset +
-			    ms->search.rm_len));
+			*o = CAST(int32_t,
+			    (ms->search.offset + ms->search.rm_len));
+		return 0;
 
 	case FILE_SEARCH:
 		if ((m->str_flags & REGEX_OFFSET_START) != 0)
-			return CAST(int32_t, ms->search.offset);
+			*o = CAST(int32_t, ms->search.offset);
 		else
-			return CAST(int32_t, (ms->search.offset + m->vallen));
+			*o = CAST(int32_t, (ms->search.offset + m->vallen));
+		return 0;
 
 	case FILE_CLEAR:
 	case FILE_DEFAULT:
 	case FILE_INDIRECT:
-		return ms->offset;
+		*o = ms->offset;
+		return 0;
+
 	case FILE_DER:
-		return der_offs(ms, m);
+		{
+			int t = der_offs(ms, m);
+			if (t == -1) {
+				file_error(ms, 0, "EOF computing DER offset");
+				return -1;
+			}
+			*o = t;
+			return 0;
+		}
 
 	default:
 		return 0;