[exim-cvs] Lookups: Fix dnsdb lookup of multi-chunk TXT. Bu…

Commit:     79670d3c32ccb37fe06f25d8192943b58606a32a
Parent:     9643095d7cae6866b716d361a45c9a95f605040c
Author:     Jeremy Harris <jgh146exb@???>
AuthorDate: Fri Nov 17 16:55:17 2023 +0000
Committer:  Jeremy Harris <jgh146exb@???>
CommitDate: Fri Nov 17 16:55:17 2023 +0000

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

    Broken=by: f6b1f8e7d642
---
 doc/doc-txt/ChangeLog        |  6 +++++-
 src/src/dns.c                |  4 +++-
 src/src/lookups/dnsdb.c      | 45 +++++++++++++++++++-------------------------
 test/dnszones-src/db.test.ex |  1 +
 test/scripts/2200-dnsdb/2200 |  1 +
 test/src/fakens.c            |  1 +
 test/stdout/2200             |  1 +
 7 files changed, 31 insertions(+), 28 deletions(-)

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 9d23e8db2..1663f8552 100644
--- 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.
+

 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 ff51dec23..8288896ca 100644
--- 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 6ff1a6af4..94ac98608 100644
--- 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 96c8dc7ec..4b89a7338 100644
--- 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 2c82c5a82..0de4e60c2 100644
--- 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 766d433ef..8d02a903e 100644
--- 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