Gitweb:
https://git.exim.org/exim.git/commitdiff/b94ea1bd61485a97c2d0dc2cab4c4d86ffe82e89
Commit: b94ea1bd61485a97c2d0dc2cab4c4d86ffe82e89
Parent: bb6a0f57e5c1a04c9c191af6a37184970003b1c2
Author: Jeremy Harris <jgh146exb@???>
AuthorDate: Sun Oct 15 12:15:06 2023 +0100
Committer: Jeremy Harris <jgh146exb@???>
CommitDate: Thu Oct 19 22:55:08 2023 +0100
DNS: more hardening against crafted responses
---
src/src/acl.c | 1 +
src/src/dns.c | 39 ++++++++++++++++++++++++++++++---------
src/src/functions.h | 16 ++++++++++++++++
src/src/host.c | 3 +++
src/src/lookups/dnsdb.c | 10 +++++-----
5 files changed, 55 insertions(+), 14 deletions(-)
diff --git a/src/src/acl.c b/src/src/acl.c
index 118e4b35d..302dedaeb 100644
--- 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 db566f2e8..1347deec8 100644
--- 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 8f85165e7..39119ca09 100644
--- 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 3e5a88660..ce7ca2bab 100644
--- 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 35a946447..fcf80e3dd 100644
--- 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);
--
## subscription configuration (requires account):
##
https://lists.exim.org/mailman3/postorius/lists/exim-cvs.lists.exim.org/
## unsubscribe (doesn't require an account):
## exim-cvs-unsubscribe@???
## Exim details at
http://www.exim.org/
## Please use the Wiki with this list -
http://wiki.exim.org/
