From 152481a026745132f8cd90fac166e6f4ecb6ea58 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Sun, 10 Nov 2019 13:17:43 +0000 Subject: [PATCH] Fix taint-handling in rDNS name construction --- src/src/acl.c | 3 +-- src/src/dns.c | 27 ++++++++++++++------------- src/src/functions.h | 2 +- src/src/host.c | 8 ++++---- src/src/lookups/dnsdb.c | 6 +----- 5 files changed, 21 insertions(+), 25 deletions(-) diff --git a/src/src/acl.c b/src/src/acl.c index 8e34513d0..799af39c3 100644 --- a/src/src/acl.c +++ b/src/src/acl.c @@ -1345,8 +1345,7 @@ extension to CSA, so we allow it to be turned off for proper conformance. */ if (string_is_ip_address(domain, NULL) != 0) { if (!dns_csa_use_reverse) return CSA_UNKNOWN; - dns_build_reverse(domain, target); - domain = target; + domain = dns_build_reverse(domain); } /* Find out if we've already done the CSA check for this domain. If we have, diff --git a/src/src/dns.c b/src/src/dns.c index 7736a2204..6333d3cff 100644 --- a/src/src/dns.c +++ b/src/src/dns.c @@ -221,16 +221,15 @@ a name that can be used to look up PTR records. Arguments: string the IP address as a string - buffer a suitable buffer, long enough to hold the result -Returns: nothing +Returns: an allocated string */ -void -dns_build_reverse(const uschar *string, uschar *buffer) +uschar * +dns_build_reverse(const uschar * string) { -const uschar *p = string + Ustrlen(string); -uschar *pp = buffer; +const uschar * p = string + Ustrlen(string); +gstring * g = NULL; /* Handle IPv4 address */ @@ -240,14 +239,13 @@ if (Ustrchr(string, ':') == NULL) { for (int i = 0; i < 4; i++) { - const uschar *ppp = p; + const uschar * ppp = p; while (ppp > string && ppp[-1] != '.') ppp--; - Ustrncpy(pp, ppp, p - ppp); - pp += p - ppp; - *pp++ = '.'; + g = string_catn(g, ppp, p - ppp); + g = string_catn(g, US".", 1); p = ppp - 1; } - Ustrcpy(pp, US"in-addr.arpa"); + g = string_catn(g, US"in-addr.arpa", 12); } /* Handle IPv6 address; convert to binary so as to fill out any @@ -257,6 +255,8 @@ abbreviation in the textual form. */ else { int v6[4]; + + g = string_get_tainted(32, is_tainted(string)); (void)host_aton(string, v6); /* The original specification for IPv6 reverse lookup was to invert each @@ -265,8 +265,8 @@ else for (int i = 3; i >= 0; i--) for (int j = 0; j < 32; j += 4) - pp += sprintf(CS pp, "%x.", (v6[i] >> j) & 15); - Ustrcpy(pp, US"ip6.arpa."); + g = string_fmt_append(g, "%x.", (v6[i] >> j) & 15); + g = string_catn(g, US"ip6.arpa.", 9); /* Another way of doing IPv6 reverse lookups was proposed in conjunction with A6 records. However, it fell out of favour when they did. The @@ -290,6 +290,7 @@ else } #endif +return string_from_gstring(g); } diff --git a/src/src/functions.h b/src/src/functions.h index eca8e798d..f4fcd1e19 100644 --- a/src/src/functions.h +++ b/src/src/functions.h @@ -197,7 +197,7 @@ extern BOOL dkim_transport_write_message(transport_ctx *, #endif extern dns_address *dns_address_from_rr(dns_answer *, dns_record *); extern int dns_basic_lookup(dns_answer *, const uschar *, int); -extern void dns_build_reverse(const uschar *, uschar *); +extern uschar *dns_build_reverse(const uschar *); extern time_t dns_expire_from_soa(dns_answer *, int); extern void dns_init(BOOL, BOOL, BOOL); extern BOOL dns_is_aa(const dns_answer *); diff --git a/src/src/host.c b/src/src/host.c index aa142eb9b..3361d5912 100644 --- a/src/src/host.c +++ b/src/src/host.c @@ -1646,7 +1646,6 @@ int old_pool, rc; int sep = 0; uschar *save_hostname; uschar **aliases; -uschar buffer[256]; uschar *ordername; const uschar *list = host_lookup_order; dns_answer * dnsa = store_get_dns_answer(); @@ -1672,13 +1671,14 @@ if (f.running_in_test_harness && /* Do lookups directly in the DNS or via gethostbyaddr() (or equivalent), in the order specified by the host_lookup_order option. */ -while ((ordername = string_nextinlist(&list, &sep, buffer, sizeof(buffer)))) +while ((ordername = string_nextinlist(&list, &sep, NULL, 0))) { if (strcmpic(ordername, US"bydns") == 0) { + uschar * name = dns_build_reverse(sender_host_address); + dns_init(FALSE, FALSE, FALSE); /* dnssec ctrl by dns_dnssec_ok glbl */ - dns_build_reverse(sender_host_address, buffer); - rc = dns_lookup_timerwrap(dnsa, buffer, T_PTR, NULL); + rc = dns_lookup_timerwrap(dnsa, name, T_PTR, NULL); /* The first record we come across is used for the name; others are considered to be aliases. We have to scan twice, in order to find out the diff --git a/src/src/lookups/dnsdb.c b/src/src/lookups/dnsdb.c index 1cf8df739..94809e5d8 100644 --- a/src/src/lookups/dnsdb.c +++ b/src/src/lookups/dnsdb.c @@ -312,7 +312,6 @@ if (!outsep2) switch(type) while ((domain = string_nextinlist(&keystring, &sep, NULL, 0))) { - uschar rbuffer[256]; int searchtype = type == T_CSA ? T_SRV : /* record type we want */ type == T_MXH ? T_MX : type == T_ZNS ? T_NS : type; @@ -325,10 +324,7 @@ while ((domain = string_nextinlist(&keystring, &sep, NULL, 0))) if ((type == T_PTR || type == T_CSA) && string_is_ip_address(domain, NULL) != 0) - { - dns_build_reverse(domain, rbuffer); - domain = rbuffer; - } + domain = dns_build_reverse(domain); do { -- 2.25.1