DNS: more hardening against crafted responses

diff --git a/src/src/acl.c b/src/src/acl.c
index 118e4b35db422c18326cac1727346464685a7158..302dedaeb21fdde5f982fd5613701b04810a13f2 100644 (file)
--- a/src/src/acl.c
+++ b/src/src/acl.c
@@ -1434,6 +1434,7 @@ for (rr = dns_next_rr(dnsa, &dnss, RESET_ANSWERS);
 
   /* Extract the numerical SRV fields (p is incremented) */
 
+  if (rr_bad_size(rr, 3 * sizeof(uint16_t))) continue;
   GETSHORT(priority, p);
   GETSHORT(weight, p);
   GETSHORT(port, p);

diff --git a/src/src/dns.c b/src/src/dns.c
index db566f2e86b54a0840d9a229e3aca3a262994beb..1347deec8ac74d94391dd9c89f18193a5435d719 100644 (file)
--- a/src/src/dns.c
+++ b/src/src/dns.c
@@ -299,13 +299,23 @@ return string_from_gstring(g);
 
 
 
+/* Check a pointer for being past the end of a dns answer.
+Exactly one past the end is defined as ok.
+Return TRUE iff bad.
+*/
+static BOOL
+dnsa_bad_ptr(const dns_answer * dnsa, const uschar * ptr)
+{
+return ptr > dnsa->answer + dnsa->answerlen;
+}
+
 /* Increment the aptr in dnss, checking against dnsa length.
 Return: TRUE for a bad result
 */
 static BOOL
 dnss_inc_aptr(const dns_answer * dnsa, dns_scan * dnss, unsigned delta)
 {
-return (dnss->aptr += delta) > dnsa->answer + dnsa->answerlen;
+return dnsa_bad_ptr(dnsa, dnss->aptr += delta);
 }
 
 /*************************************************
@@ -385,10 +395,14 @@ if (reset != RESET_NEXT)
       namelen = dn_expand(dnsa->answer, dnsa->answer + dnsa->answerlen,
         dnss->aptr, (DN_EXPAND_ARG4_TYPE) &dnss->srr.name, DNS_MAXNAME);
       if (namelen < 0) goto null_return;
+
       /* skip name, type, class & TTL */
       TRACE trace = "A-hdr";
       if (dnss_inc_aptr(dnsa, dnss, namelen+8)) goto null_return;
+
+      if (dnsa_bad_ptr(dnsa, dnss->aptr + sizeof(uint16_t))) goto null_return;
       GETSHORT(dnss->srr.size, dnss->aptr); /* size of data portion */
+
       /* skip over it, checking for a bogus size */
       TRACE trace = "A-skip";
       if (dnss_inc_aptr(dnsa, dnss, dnss->srr.size)) goto null_return;
@@ -422,12 +436,18 @@ from the following bytes. */
 TRACE trace = "R-name";
 if (dnss_inc_aptr(dnsa, dnss, namelen)) goto null_return;
 
-GETSHORT(dnss->srr.type, dnss->aptr);          /* Record type */
+/* Check space for type, class, TTL & data-size-word */
+if (dnsa_bad_ptr(dnsa, dnss->aptr + 3 * sizeof(uint16_t) + sizeof(uint32_t)))
+  goto null_return;
+
+GETSHORT(dnss->srr.type, dnss->aptr);                  /* Record type */
+
 TRACE trace = "R-class";
-if (dnss_inc_aptr(dnsa, dnss, 2)) goto null_return;    /* Don't want class */
-GETLONG(dnss->srr.ttl, dnss->aptr);            /* TTL */
-GETSHORT(dnss->srr.size, dnss->aptr);          /* Size of data portion */
-dnss->srr.data = dnss->aptr;                   /* The record's data follows */
+(void) dnss_inc_aptr(dnsa, dnss, sizeof(uint16_t));    /* skip class */
+
+GETLONG(dnss->srr.ttl, dnss->aptr);                    /* TTL */
+GETSHORT(dnss->srr.size, dnss->aptr);                  /* Size of data portion */
+dnss->srr.data = dnss->aptr;                           /* The record's data follows */
 
 /* skip over it, checking for a bogus size */
 if (dnss_inc_aptr(dnsa, dnss, dnss->srr.size))
@@ -743,17 +763,17 @@ if (fake_dnsa_len_for_fail(dnsa, type))
     /* Skip the mname & rname strings */
 
     if ((len = dn_expand(dnsa->answer, dnsa->answer + dnsa->answerlen,
-       p, (DN_EXPAND_ARG4_TYPE)discard_buf, 256)) < 0)
+       p, (DN_EXPAND_ARG4_TYPE)discard_buf, sizeof(discard_buf))) < 0)
       break;
     p += len;
     if ((len = dn_expand(dnsa->answer, dnsa->answer + dnsa->answerlen,
-       p, (DN_EXPAND_ARG4_TYPE)discard_buf, 256)) < 0)
+       p, (DN_EXPAND_ARG4_TYPE)discard_buf, sizeof(discard_buf))) < 0)
       break;
     p += len;
 
     /* Skip the SOA serial, refresh, retry & expire.  Grab the TTL */
 
-    if (p > dnsa->answer + dnsa->answerlen - 5 * INT32SZ)
+    if (dnsa_bad_ptr(dnsa, p + 5 * INT32SZ))
       break;
     p += 4 * INT32SZ;
     GETLONG(ttl, p);
@@ -1257,6 +1277,7 @@ switch (type)
        const uschar * p = rr->data;
 
        /* Extract the numerical SRV fields (p is incremented) */
+       if (rr_bad_size(rr, 3 * sizeof(uint16_t))) continue;
        GETSHORT(priority, p);
        GETSHORT(dummy_weight, p);
        GETSHORT(port, p);

diff --git a/src/src/functions.h b/src/src/functions.h
index 8f85165e73bc5eb16b508c485bd1e1b16a27fbe2..39119ca093801e0957aad3b3d59c18b716251d25 100644 (file)
--- a/src/src/functions.h
+++ b/src/src/functions.h
@@ -1110,6 +1110,22 @@ store_free_dns_answer_trc(dns_answer * dnsa, const uschar * func, unsigned line)
 store_free_3(dnsa, CCS func, line);
 }
 
+
+/* Check for an RR being large enough.  Return TRUE iff bad. */
+static inline BOOL
+rr_bad_size(const dns_record * rr, size_t minbytes)
+{
+return rr->size < minbytes;
+}
+
+/* Check for an RR having further data beyond a given pointer.
+Return TRUE iff bad. */
+static inline BOOL
+rr_bad_increment(const dns_record * rr, const uschar * ptr, size_t minbytes)
+{
+return rr_bad_size(rr, ptr - rr->data + minbytes);
+}
+
 /******************************************************************************/
 /* Routines with knowledge of spool layout */
 

diff --git a/src/src/host.c b/src/src/host.c
index 3e5a886603390c80e881071f4692af577f20dadd..ce7ca2bab1098d6f5167cd485dd46c42d0451163 100644 (file)
--- a/src/src/host.c
+++ b/src/src/host.c
@@ -2725,6 +2725,7 @@ for (dns_record * rr = dns_next_rr(dnsa, &dnss, RESET_ANSWERS);
   const uschar * s = rr->data; /* MUST be unsigned for GETSHORT */
   uschar data[256];
 
+  if (rr_bad_size(rr, sizeof(uint16_t))) continue;
   GETSHORT(precedence, s);      /* Pointer s is advanced */
 
   /* For MX records, we use a random "weight" which causes multiple records of
@@ -2737,6 +2738,8 @@ for (dns_record * rr = dns_next_rr(dnsa, &dnss, RESET_ANSWERS);
     /* SRV records are specified with a port and a weight. The weight is used
     in a special algorithm. However, to start with, we just use it to order the
     records of equal priority (precedence). */
+
+    if (rr_bad_increment(rr, s, 2 * sizeof(uint16_t))) continue;
     GETSHORT(weight, s);
     GETSHORT(port, s);
     }

diff --git a/src/src/lookups/dnsdb.c b/src/src/lookups/dnsdb.c
index 35a9464470e983a7e71de5a4e87c4a63cc45b0ae..fcf80e3dd6983fca9dfea87f40a7377b996aa159 100644 (file)
--- a/src/src/lookups/dnsdb.c
+++ b/src/src/lookups/dnsdb.c
@@ -452,20 +452,20 @@ while ((domain = string_nextinlist(&keystring, &sep, NULL, 0)))
        switch (type)
          {
          case T_MXH:
-           if (rr->size < sizeof(u_int16_t)) continue;
+           if (rr_bad_size(rr, sizeof(u_int16_t))) continue;
            /* mxh ignores the priority number and includes only the hostnames */
            GETSHORT(priority, p);
            break;
 
          case T_MX:
-           if (rr->size < sizeof(u_int16_t)) continue;
+           if (rr_bad_size(rr, sizeof(u_int16_t))) continue;
            GETSHORT(priority, p);
            sprintf(CS s, "%d%c", priority, *outsep2);
            yield = string_cat(yield, s);
            break;
 
          case T_SRV:
-           if (rr->size < 3*sizeof(u_int16_t)) continue;
+           if (rr_bad_size(rr, 3*sizeof(u_int16_t))) continue;
            GETSHORT(priority, p);
            GETSHORT(weight, p);
            GETSHORT(port, p);
@@ -475,7 +475,7 @@ while ((domain = string_nextinlist(&keystring, &sep, NULL, 0)))
            break;
 
          case T_CSA:
-           if (rr->size < 3*sizeof(u_int16_t)) continue;
+           if (rr_bad_size(rr, 3*sizeof(u_int16_t))) continue;
            /* See acl_verify_csa() for more comments about CSA. */
            GETSHORT(priority, p);
            GETSHORT(weight, p);
@@ -542,7 +542,7 @@ while ((domain = string_nextinlist(&keystring, &sep, NULL, 0)))
          else yield = string_cat(yield, s);
 
          p += rc;
-         if (rr->size >= p - rr->data - 5*sizeof(u_int32_t))
+         if (!rr_bad_increment(rr, p, 5 * sizeof(u_int32_t)))
            {
            GETLONG(serial, p); GETLONG(refresh, p);
            GETLONG(retry,  p); GETLONG(expire,  p); GETLONG(minimum, p);