Lookups: Fix dnsdb lookup of multi-chunk TXT. Bug 3054

Broken=by: f6b1f8e7d642

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 9d23e8db2f365135bbe6e89038ec3019b9c0a8bd..1663f85527b9fb63d02a29314ea89f8b8ebc3264 100644 (file)
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -24,13 +24,17 @@ JH/04 Bug 3039: Fix handling of of an empty log_reject_target, with
       a connection_reject log_selector, under tls_on_connect.  Previously
       with this combination, when the connect ACL rejected, a spurious
       paniclog entry was made.
-JH/04 Fix TLS resumption for TLS-on-connect.  This was broken by the advent
+
+JH/05 Fix TLS resumption for TLS-on-connect.  This was broken by the advent
       of loadbalancer-detection for resumption, in 4.96 - which tries to
       use the EHLO response. SMTPS does not have one at the time it is starting
       TLS.  Change the default for the smtp transport host_name_extract option
       to be a static string, for TLS-on-connect cases; meaning that resumption
       will always be attempted (unless deliberately overriden).
 
+JH/06 Bug 3054: Fix dnsdb lookup for a TXT record with multiple chunks.  This
+      was broken by hardening introduced for Bug 3033.
+
 
 
 Exim version 4.97

diff --git a/src/src/dns.c b/src/src/dns.c
index a652dcd31cd611aac62379c0e9002cf84fa93e73..74c5a58c538df86abf3c37ea78d9db5462185313 100644 (file)
--- a/src/src/dns.c
+++ b/src/src/dns.c
@@ -431,7 +431,9 @@ namelen = dn_expand(dnsa->answer, dnsa->answer + dnsa->answerlen, dnss->aptr,
 if (namelen < 0) goto null_return;
 
 /* Move the pointer past the name and fill in the rest of the data structure
-from the following bytes. */
+from the following bytes.  We seem to be assuming here that the RR blob passed
+to us by the resolver library is the same as that defined for an RR by RFC 1035
+section 3.2.1 */
 
 TRACE trace = "R-name";
 if (dnss_inc_aptr(dnsa, dnss, namelen)) goto null_return;

diff --git a/src/src/lookups/dnsdb.c b/src/src/lookups/dnsdb.c
index ff51dec23d59cd88a09b6537f628310683fd8011..8288896ca42ae5f90e346e28906ff1391ac41b17 100644 (file)
--- a/src/src/lookups/dnsdb.c
+++ b/src/src/lookups/dnsdb.c
@@ -387,38 +387,31 @@ while ((domain = string_nextinlist(&keystring, &sep, NULL, 0)))
         }
 
       /* Other kinds of record just have one piece of data each, but there may be
-      several of them, of course. */
+      several of them, of course.  TXT & SPF can have data in multiple chunks. */
 
       if (yield->ptr) yield = string_catn(yield, outsep, 1);
 
       if (type == T_TXT || type == T_SPF)
-        {
-        if (!outsep2)                  /* output only the first item of data */
+       for (unsigned data_offset = 0; data_offset + 1 < rr->size; )
          {
-         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);
+         uschar chunk_len = (rr->data)[data_offset];
+         int remain;
+
+         if (outsep2 && *outsep2 && data_offset != 0)
+           yield = string_catn(yield, outsep2, 1);
+
+         /* 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 */
+
+         remain = rr->size - ++data_offset;
+         if (chunk_len > remain)
+           chunk_len = remain;
+         yield = string_catn(yield, US ((rr->data) + data_offset), chunk_len);
+         data_offset += chunk_len;
+
+         if (!outsep2) break;          /* output only the first chunk of the RR */
          }
-        else
-          for (unsigned data_offset = 0; data_offset < rr->size; )
-            {
-            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);
-            data_offset += chunk_len;
-            }
-        }
       else if (type == T_TLSA)
        if (rr->size < 3)
          continue;

diff --git a/test/dnszones-src/db.test.ex b/test/dnszones-src/db.test.ex
index 6ff1a6af4a26d001777a2cc0d32b6383f369bbd4..94ac9860830e3bc0bb459d96e9f2835558c5feb5 100644 (file)
--- a/test/dnszones-src/db.test.ex
+++ b/test/dnszones-src/db.test.ex
@@ -24,6 +24,7 @@ test.ex.     SOA     exim.test.ex. hostmaster.exim.test.ex 1430683638 1200 120 6
 
 test.ex.     TXT     "A TXT record for test.ex."
 s/lash       TXT     "A TXT record for s/lash.test.ex."
+long         TXT     "This is a max-length chunk 789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234" "A short chunk" "A final chunk"
 
 cname        CNAME   test.ex.
 

diff --git a/test/scripts/2200-dnsdb/2200 b/test/scripts/2200-dnsdb/2200
index 96c8dc7ecfa7582f200529d874e746fdd1c19c39..4b89a733825c74af9a441e1d4132c4d70cbf5b26 100644 (file)
--- a/test/scripts/2200-dnsdb/2200
+++ b/test/scripts/2200-dnsdb/2200
@@ -4,6 +4,7 @@ exim -be
 test.ex                    ${lookup dnsdb{test.ex}{$value}fail}
 s/lash.test.ex             ${lookup dnsdb{s/lash.test.ex}{$value}fail}
 txt=test.ex                ${lookup dnsdb{txt=test.ex}{$value}fail}
+txt=long.test.ex           ${lookup dnsdb{>\n; txt=long.test.ex}{$value}fail}
 a=black-1.test.ex          ${lookup dnsdb{a=black-1.test.ex}{$value}fail}
 xxx=test.ex                ${lookup dnsdb{xxx=test.ex}{$value}fail}
 a=localhost.test.ex        ${lookup dnsdb{a=localhost.test.ex}{$value}fail}

diff --git a/test/src/fakens.c b/test/src/fakens.c
index 2c82c5a82858504ea1c486a604d2a52dd87c7865..0de4e60c29b2a6590de140d6e28f67b0c4bb87e6 100644 (file)
--- a/test/src/fakens.c
+++ b/test/src/fakens.c
@@ -541,6 +541,7 @@ while (fgets(CS buffer, sizeof(buffer), f) != NULL)
       while (*p != 0 && *p != '"') *pk++ = *p++;
       *pp = pk - pp - 1;
       break;
+      /*XXX need a way of doing multi-chunk TXT RRs */
 
     case ns_t_tlsa:
       pk = bytefield(&p, pk);   /* usage */

diff --git a/test/stdout/2200 b/test/stdout/2200
index 766d433ef174e255613d77c4b644883bb5aa4d14..8d02a903e73990dbbd85ab0d93bf2ce005a79db9 100644 (file)
--- a/test/stdout/2200
+++ b/test/stdout/2200
@@ -1,6 +1,7 @@
 > test.ex                    A TXT record for test.ex.
 > s/lash.test.ex             A TXT record for s/lash.test.ex.
 > txt=test.ex                A TXT record for test.ex.
+> txt=long.test.ex           This is a max-length chunk 789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234A short chunkA final chunk
 > a=black-1.test.ex          V4NET.11.12.13
 > Failed: lookup of "xxx=test.ex" gave DEFER: unsupported DNS record type
 > a=localhost.test.ex        127.0.0.1