Fix crash from SRV lookup hitting a CNAME
[exim.git] / src / src / dns.c
index 83de7c26684ea3a6ad640089899c59e986dcb453..6ef6b7784b03343bff3a2b3fae635577d8300d37 100644 (file)
@@ -240,8 +240,7 @@ uschar *pp = buffer;
 if (Ustrchr(string, ':') == NULL)
 #endif
   {
-  int i;
-  for (i = 0; i < 4; i++)
+  for (int i = 0; i < 4; i++)
     {
     const uschar *ppp = p;
     while (ppp > string && ppp[-1] != '.') ppp--;
@@ -259,7 +258,6 @@ abbreviation in the textual form. */
 #if HAVE_IPV6
 else
   {
-  int i;
   int v6[4];
   (void)host_aton(string, v6);
 
@@ -267,12 +265,9 @@ else
   nibble, and look in the ip6.int domain. The domain was subsequently
   changed to ip6.arpa. */
 
-  for (i = 3; i >= 0; i--)
-    {
-    int j;
-    for (j = 0; j < 32; j += 4)
+  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, "ip6.arpa.");
 
   /* Another way of doing IPv6 reverse lookups was proposed in conjunction
@@ -287,7 +282,7 @@ else
   Ustrcpy(pp, "\\[x");
   pp += 3;
 
-  for (i = 0; i < 4; i++)
+  for (int i = 0; i < 4; i++)
     {
     sprintf(pp, "%08X", v6[i]);
     pp += 8;
@@ -467,11 +462,10 @@ static const uschar *
 dns_extract_auth_name(const dns_answer * dnsa) /* FIXME: const dns_answer */
 {
 dns_scan dnss;
-dns_record * rr;
 const HEADER * h = (const HEADER *) dnsa->answer;
 
 if (h->nscount && h->aa)
-  for (rr = dns_next_rr(dnsa, &dnss, RESET_AUTHORITY);
+  for (dns_record * rr = dns_next_rr(dnsa, &dnss, RESET_AUTHORITY);
        rr; rr = dns_next_rr(dnsa, &dnss, RESET_NEXT))
     if (rr->type == (h->ancount ? T_NS : T_SOA))
       return string_copy(rr->name);
@@ -639,6 +633,10 @@ up nameservers that produce this error continually, so there is the option of
 providing a list of domains for which this is treated as a non-existent
 host.
 
+The dns_answer structure is pretty big; enough to hold a max-sized DNS message
+- so best allocated from fast-release memory.  As of writing, all our callers
+use a stack-auto variable.
+
 Arguments:
   dnsa      pointer to dns_answer structure
   name      name to look up
@@ -674,10 +672,10 @@ if ((previous = tree_search(tree_dns_fails, node_name)))
   {
   DEBUG(D_dns) debug_printf("DNS lookup of %.255s-%s: using cached value %s\n",
     name, dns_text_type(type),
-      (previous->data.val == DNS_NOMATCH)? "DNS_NOMATCH" :
-      (previous->data.val == DNS_NODATA)? "DNS_NODATA" :
-      (previous->data.val == DNS_AGAIN)? "DNS_AGAIN" :
-      (previous->data.val == DNS_FAIL)? "DNS_FAIL" : "??");
+      previous->data.val == DNS_NOMATCH ? "DNS_NOMATCH" :
+      previous->data.val == DNS_NODATA ? "DNS_NODATA" :
+      previous->data.val == DNS_AGAIN ? "DNS_AGAIN" :
+      previous->data.val == DNS_FAIL ? "DNS_FAIL" : "??");
   return previous->data.val;
   }
 
@@ -693,7 +691,7 @@ if ((previous = tree_search(tree_dns_fails, node_name)))
     DEBUG(D_dns)
       debug_printf("DNS name '%s' utf8 conversion to alabel failed: %s\n", name,
         errstr);
-    host_find_failed_syntax = TRUE;
+    f.host_find_failed_syntax = TRUE;
     return DNS_NOMATCH;
     }
   name = alabel;
@@ -712,7 +710,11 @@ lookup, which constructs the names itself, so they should be OK. Besides,
 bitstring labels don't conform to normal name syntax. (But the aren't used any
 more.)
 
-For SRV records, we omit the initial _smtp._tcp. components at the start. */
+For SRV records, we omit the initial _smtp._tcp. components at the start.
+The check has been seen to bite on the destination of a SRV lookup that
+initiall hit a CNAME, for which the next name had only two components.
+RFC2782 makes no mention of the possibiility of CNAMES, but the Wikipedia
+article on SRV says they are not a valid configuration. */
 
 #ifndef STAND_ALONE   /* Omit this for stand-alone tests */
 
@@ -728,8 +730,8 @@ if (check_dns_names_pattern[0] != 0 && type != T_PTR && type != T_TXT)
 
   if (type == T_SRV || type == T_TLSA)
     {
-    while (*checkname++ != '.');
-    while (*checkname++ != '.');
+    while (*checkname && *checkname++ != '.') ;
+    while (*checkname && *checkname++ != '.') ;
     }
 
   if (pcre_exec(regex_check_dns_names, NULL, CCS checkname, Ustrlen(checkname),
@@ -738,7 +740,7 @@ if (check_dns_names_pattern[0] != 0 && type != T_PTR && type != T_TXT)
     DEBUG(D_dns)
       debug_printf("DNS name syntax check failed: %s (%s)\n", name,
         dns_text_type(type));
-    host_find_failed_syntax = TRUE;
+    f.host_find_failed_syntax = TRUE;
     return DNS_NOMATCH;
     }
   }
@@ -761,7 +763,7 @@ if ((type == T_A || type == T_AAAA) && string_is_ip_address(name, NULL) != 0)
 (res_search), we call fakens_search(), which recognizes certain special
 domains, and interfaces to a fake nameserver for certain special zones. */
 
-dnsa->answerlen = running_in_test_harness
+dnsa->answerlen = f.running_in_test_harness
   ? fakens_search(name, type, dnsa->answer, sizeof(dnsa->answer))
   : res_search(CCS name, C_IN, type, dnsa->answer, sizeof(dnsa->answer));
 
@@ -836,6 +838,8 @@ return DNS_SUCCEED;
 /* Look up the given domain name, using the given type. Follow CNAMEs if
 necessary, but only so many times. There aren't supposed to be CNAME chains in
 the DNS, but you are supposed to cope with them if you find them.
+By default, follow one CNAME since a resolver has been seen, faced with
+an MX request and a CNAME (to an A) but no MX present, returning the CNAME.
 
 The assumption is made that if the resolver gives back records of the
 requested type *and* a CNAME, we don't need to make another call to look up
@@ -867,18 +871,22 @@ int
 dns_lookup(dns_answer *dnsa, const uschar *name, int type,
   const uschar **fully_qualified_name)
 {
-int i;
 const uschar *orig_name = name;
 BOOL secure_so_far = TRUE;
 
-/* Loop to follow CNAME chains so far, but no further... */
+/* By default, assume the resolver follows CNAME chains (and returns NODATA for
+an unterminated one). If it also does that for a CNAME loop, fine; if it returns
+a CNAME (maybe the last?) whine about it.  However, retain the coding for dumb
+resolvers hiding behind a config variable. Loop to follow CNAME chains so far,
+but no further...  The testsuite tests the latter case, mostly assuming that the
+former will work. */
 
-for (i = 0; i < 10; i++)
+for (int i = 0; i <= dns_cname_loops; i++)
   {
   uschar * data;
-  dns_record *rr, cname_rr, type_rr;
+  dns_record cname_rr, type_rr;
   dns_scan dnss;
-  int datalen, rc;
+  int rc;
 
   /* DNS lookup failures get passed straight back. */
 
@@ -892,7 +900,7 @@ for (i = 0; i < 10; i++)
   area in the dnsa block. */
 
   cname_rr.data = type_rr.data = NULL;
-  for (rr = dns_next_rr(dnsa, &dnss, RESET_ANSWERS);
+  for (dns_record * rr = dns_next_rr(dnsa, &dnss, RESET_ANSWERS);
        rr; rr = dns_next_rr(dnsa, &dnss, RESET_NEXT))
     if (rr->type == type)
       {
@@ -940,8 +948,8 @@ for (i = 0; i < 10; i++)
     return DNS_FAIL;
 
   data = store_get(256);
-  if ((datalen = dn_expand(dnsa->answer, dnsa->answer + dnsa->answerlen,
-    cname_rr.data, (DN_EXPAND_ARG4_TYPE)data, 256)) < 0)
+  if (dn_expand(dnsa->answer, dnsa->answer + dnsa->answerlen,
+      cname_rr.data, (DN_EXPAND_ARG4_TYPE)data, 256) < 0)
     return DNS_FAIL;
   name = data;
 
@@ -1094,7 +1102,7 @@ switch (type)
          && (h->rcode == NOERROR || h->rcode == NXDOMAIN)
          && ntohs(h->qdcount) == 1 && ntohs(h->ancount) == 0
          && ntohs(h->nscount) >= 1)
-           dnsa->answerlen = MAXPACKET;
+           dnsa->answerlen = sizeof(dnsa->answer);
 
       for (rr = dns_next_rr(dnsa, &dnss, RESET_AUTHORITY);
           rr; rr = dns_next_rr(dnsa, &dnss, RESET_NEXT)
@@ -1206,8 +1214,7 @@ else
   if (rr->data + 16 <= dnsa_lim)
     {
     struct in6_addr in6;
-    int i;
-    for (i = 0; i < 16; i++) in6.s6_addr[i] = rr->data[i];
+    for (int i = 0; i < 16; i++) in6.s6_addr[i] = rr->data[i];
     yield = store_get(sizeof(dns_address) + 50);
     inet_ntop(AF_INET6, &in6, CS yield->address, 50);
     yield->next = NULL;