Add bounds checks to DER code offset computations.

diff --git a/ChangeLog b/ChangeLog
index 24c06c8a569492596bc2e59f9fc9ed7fc652e30e..75730b1d06a0905a1b3b026ec7f54c0b068233f3 100644 (file)
--- 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 b1784fd59c7e9a79cdf44d928791a6fb1f51134d..80ebe2cdbc09b9496b717ec07421d127f7c9735b 100644 (file)
--- a/src/der.c
+++ b/src/der.c
@@ -35,7 +35,7 @@
 #include "file.h"
 
 #ifndef lint
-FILE_RCSID("@(#)$File: der.c,v 1.1 2016/01/19 04:17:07 christos Exp $")
+FILE_RCSID("@(#)$File: der.c,v 1.2 2016/01/19 15:09:28 christos Exp $")
 #endif
 #endif
 
@@ -55,6 +55,8 @@ FILE_RCSID("@(#)$File: der.c,v 1.1 2016/01/19 04:17:07 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 3a785847bf66f2116c6a5cba548e8d678656a2ca..618f6e24c4178bc66fab5b71c257db79a5d8e1fa 100644 (file)
--- a/src/softmagic.c
+++ b/src/softmagic.c
@@ -32,7 +32,7 @@
 #include "file.h"
 
 #ifndef        lint
-FILE_RCSID("@(#)$File: softmagic.c,v 1.226 2016/03/02 18:27:53 christos Exp $")
+FILE_RCSID("@(#)$File: softmagic.c,v 1.227 2016/03/21 15:56:53 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;