Gitweb:
https://git.exim.org/exim.git/commitdiff/f6b1f8e7d642f82d830a71b78699a4349e0158e1
Commit: f6b1f8e7d642f82d830a71b78699a4349e0158e1
Parent: 654056e44fc93a0ee7c09d1228933e8af6862206
Author: Jeremy Harris <jgh146exb@???>
AuthorDate: Tue Oct 10 23:03:28 2023 +0100
Committer: Heiko Schlittermann (HS12-RIPE) <hs@???>
CommitDate: Sat Oct 14 23:50:37 2023 +0200
Harden dnsdb against crafted DNS responses. Bug 3033
(cherry picked from commit 8787c8994f07c23c3664d76926e02f07314d699d)
---
doc/doc-txt/ChangeLog | 3 ++
src/src/dns.c | 11 ++++---
src/src/lookups/dnsdb.c | 78 ++++++++++++++++++++++++++++++++-----------------
3 files changed, 59 insertions(+), 33 deletions(-)
diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index c36718d7e..f7ab3c005 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -225,6 +225,9 @@ JH/39 Bug 3023: Fix crash induced by some combinations of zero-length strings
JH/40 Support list of dkim results in the dkim_status ACL condition, making
it more usable in the data ACL.
+JH/43 Bug 3033: Harden dnsdb lookups against crafted DNS responses.
+ CVE-2023-42219
+
Exim version 4.96
-----------------
diff --git a/src/src/dns.c b/src/src/dns.c
index 7d7ee0c04..8dc3695a1 100644
--- a/src/src/dns.c
+++ b/src/src/dns.c
@@ -304,7 +304,7 @@ 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 (dnss->aptr += delta) > dnsa->answer + dnsa->answerlen;
}
/*************************************************
@@ -388,7 +388,7 @@ if (reset != RESET_NEXT)
TRACE trace = "A-hdr";
if (dnss_inc_aptr(dnsa, dnss, namelen+8)) goto null_return;
GETSHORT(dnss->srr.size, dnss->aptr); /* size of data portion */
- /* skip over it */
+ /* skip over it, checking for a bogus size */
TRACE trace = "A-skip";
if (dnss_inc_aptr(dnsa, dnss, dnss->srr.size)) goto null_return;
}
@@ -428,10 +428,9 @@ 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 */
-/* Unchecked increment ok here since no further access on this iteration;
-will be checked on next at "R-name". */
-
-dnss->aptr += dnss->srr.size; /* Advance to next RR */
+/* skip over it, checking for a bogus size */
+if (dnss_inc_aptr(dnsa, dnss, dnss->srr.size))
+ goto null_return;
/* Return a pointer to the dns_record structure within the dns_answer. This is
for convenience so that the scans can use nice-looking for loops. */
diff --git a/src/src/lookups/dnsdb.c b/src/src/lookups/dnsdb.c
index 355be1b5d..020dc9a52 100644
--- a/src/src/lookups/dnsdb.c
+++ b/src/src/lookups/dnsdb.c
@@ -398,43 +398,60 @@ while ((domain = string_nextinlist(&keystring, &sep, NULL, 0)))
if (type == T_TXT || type == T_SPF)
{
- if (outsep2 == NULL) /* output only the first item of data */
- yield = string_catn(yield, US (rr->data+1), (rr->data)[0]);
+ if (!outsep2) /* output only the first item of data */
+ {
+ uschar n = (rr->data)[0];
+ /* size byte + data bytes must not excced the RRs length */
+ if (n + 1 <= rr->size)
+ yield = string_catn(yield, US (rr->data+1), n);
+ }
else
{
/* output all items */
int data_offset = 0;
while (data_offset < rr->size)
{
- uschar chunk_len = (rr->data)[data_offset++];
- if (outsep2[0] != '\0' && data_offset != 1)
+ uschar chunk_len = (rr->data)[data_offset];
+ int remain = rr->size - data_offset;
+
+ /* Apparently there are resolvers that do not check RRs before passing
+ them on, and glibc fails to do so. So every application must...
+ Check for chunk len exceeding RR */
+
+ if (chunk_len > remain)
+ chunk_len = remain;
+
+ if (*outsep2 && data_offset != 0)
yield = string_catn(yield, outsep2, 1);
- yield = string_catn(yield, US ((rr->data)+data_offset), chunk_len);
+ yield = string_catn(yield, US ((rr->data) + ++data_offset), --chunk_len);
data_offset += chunk_len;
}
}
}
else if (type == T_TLSA)
- {
- uint8_t usage, selector, matching_type;
- uint16_t payload_length;
- uschar s[MAX_TLSA_EXPANDED_SIZE];
- uschar * sp = s;
- uschar * p = US rr->data;
+ if (rr->size < 3)
+ continue;
+ else
+ {
+ uint8_t usage, selector, matching_type;
+ uint16_t payload_length;
+ uschar s[MAX_TLSA_EXPANDED_SIZE];
+ uschar * sp = s;
+ uschar * p = US rr->data;
+
+ usage = *p++;
+ selector = *p++;
+ matching_type = *p++;
+ /* What's left after removing the first 3 bytes above */
+ payload_length = rr->size - 3;
+ sp += sprintf(CS s, "%d%c%d%c%d%c", usage, *outsep2,
+ selector, *outsep2, matching_type, *outsep2);
+ /* Now append the cert/identifier, one hex char at a time */
+ while (payload_length-- > 0 && sp-s < (MAX_TLSA_EXPANDED_SIZE - 4))
+ sp += sprintf(CS sp, "%02x", *p++);
- usage = *p++;
- selector = *p++;
- matching_type = *p++;
- /* What's left after removing the first 3 bytes above */
- payload_length = rr->size - 3;
- sp += sprintf(CS s, "%d%c%d%c%d%c", usage, *outsep2,
- selector, *outsep2, matching_type, *outsep2);
- /* Now append the cert/identifier, one hex char at a time */
- while (payload_length-- > 0 && sp-s < (MAX_TLSA_EXPANDED_SIZE - 4))
- sp += sprintf(CS sp, "%02x", *p++);
-
- yield = string_cat(yield, s);
- }
+ yield = string_cat(yield, s);
+ }
else /* T_CNAME, T_CSA, T_MX, T_MXH, T_NS, T_PTR, T_SOA, T_SRV */
{
int priority, weight, port;
@@ -444,17 +461,20 @@ while ((domain = string_nextinlist(&keystring, &sep, NULL, 0)))
switch (type)
{
case T_MXH:
+ if (rr->size < 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;
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;
GETSHORT(priority, p);
GETSHORT(weight, p);
GETSHORT(port, p);
@@ -464,6 +484,7 @@ while ((domain = string_nextinlist(&keystring, &sep, NULL, 0)))
break;
case T_CSA:
+ if (rr->size < 3*sizeof(u_int16_t)) continue;
/* See acl_verify_csa() for more comments about CSA. */
GETSHORT(priority, p);
GETSHORT(weight, p);
@@ -514,7 +535,7 @@ while ((domain = string_nextinlist(&keystring, &sep, NULL, 0)))
if (type == T_SOA && outsep2 != NULL)
{
- unsigned long serial, refresh, retry, expire, minimum;
+ unsigned long serial = 0, refresh = 0, retry = 0, expire = 0, minimum = 0;
p += rc;
yield = string_catn(yield, outsep2, 1);
@@ -530,8 +551,11 @@ while ((domain = string_nextinlist(&keystring, &sep, NULL, 0)))
else yield = string_cat(yield, s);
p += rc;
- GETLONG(serial, p); GETLONG(refresh, p);
- GETLONG(retry, p); GETLONG(expire, p); GETLONG(minimum, p);
+ if (rr->size >= p - rr->data - 5*sizeof(u_int32_t))
+ {
+ GETLONG(serial, p); GETLONG(refresh, p);
+ GETLONG(retry, p); GETLONG(expire, p); GETLONG(minimum, p);
+ }
sprintf(CS s, "%c%lu%c%lu%c%lu%c%lu%c%lu",
*outsep2, serial, *outsep2, refresh,
*outsep2, retry, *outsep2, expire, *outsep2, minimum);
--
## 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/
