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;