Fix crash associated with dnsdb lookup done from DKIM ACL. Bug 2215

Broken-by: cc55f4208e

6 files changed, 85 insertions, 18 deletions

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 7ec669b1c..7d1d526d7 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -25,6 +25,18 @@ JH/04 Bug 2217: Tighten up the parsing of DKIM signature headers. Previously
       Assumptions at that stage could crash the receive process on malformed
       input.
 
+JH/05 Bug 2215: Fix crash associated with dnsdb lookup done from DKIM ACL.
+      While running the DKIM ACL we operate on the Permanent memory pool so that
+      variables created with "set" persist to the DATA ACL.  Also (at any time)
+      DNS lookups that fail create cache records using the Permanent pool.  But
+      expansions release any allocations made on the current pool - so a dnsdb
+      lookup expansion done in the DKIM ACL releases the memory used for the
+      DNS negative-cache, and bad things result.  Solution is to switch to the
+      Main pool for expansions.
+      While we're in that code, add checks on the DNS cache during store_reset,
+      active in the testsuite.
+      Problem spotted, and debugging aided, by Wolfgang Breyha.
+
 
 Exim version 4.90
 -----------------
diff --git a/src/src/expand.c b/src/src/expand.c
index e754fbc8c..3a63a7c78 100644
--- a/src/src/expand.c
+++ b/src/src/expand.c
@@ -7546,7 +7546,7 @@ DEBUG(D_expand)
   if (expand_string_forcedfail)
     debug_printf_indent(UTF8_UP_RIGHT "failure was forced\n");
   }
-if (resetok_p) *resetok_p = resetok;
+if (resetok_p && !resetok) *resetok_p = FALSE;
 expand_level--;
 return NULL;
 }
@@ -7560,28 +7560,35 @@ Returns:  the expanded string, or NULL if expansion failed; if failure was
           due to a lookup deferring, search_find_defer will be TRUE
 */
 
-uschar *
-expand_string(uschar *string)
+const uschar *
+expand_cstring(const uschar * string)
 {
-search_find_defer = FALSE;
-malformed_header = FALSE;
-return (Ustrpbrk(string, "$\\") == NULL)? string :
-  expand_string_internal(string, FALSE, NULL, FALSE, TRUE, NULL);
+if (Ustrpbrk(string, "$\\") != NULL)
+  {
+  int old_pool = store_pool;
+  uschar * s;
+
+  search_find_defer = FALSE;
+  malformed_header = FALSE;
+  store_pool = POOL_MAIN;
+    s = expand_string_internal(string, FALSE, NULL, FALSE, TRUE, NULL);
+  store_pool = old_pool;
+  return s;
+  }
+return string;
 }
 
 
-
-const uschar *
-expand_cstring(const uschar *string)
+uschar *
+expand_string(uschar * string)
 {
-search_find_defer = FALSE;
-malformed_header = FALSE;
-return (Ustrpbrk(string, "$\\") == NULL)? string :
-  expand_string_internal(string, FALSE, NULL, FALSE, TRUE, NULL);
+return US expand_cstring(CUS string);
 }
 
 
 
+
+
 /*************************************************
 *              Expand and copy                   *
 *************************************************/
@@ -7812,8 +7819,6 @@ return (  (  Ustrstr(s, "failed to expand") != NULL
 * Error-checking for testsuite                   *
 *************************************************/
 typedef struct {
-  const char *	filename;
-  int		linenumber;
   uschar * 	region_start;
   uschar *	region_end;
   const uschar *var_name;
@@ -7834,7 +7839,8 @@ if (var_data >= e->region_start  &&  var_data < e->region_end)
 void
 assert_no_variables(void * ptr, int len, const char * filename, int linenumber)
 {
-err_ctx e = {filename, linenumber, ptr, US ptr + len, NULL };
+err_ctx e = { .region_start = ptr, .region_end = US ptr + len,
+	      .var_name = NULL, .var_data = NULL };
 int i;
 var_entry * v;
 
@@ -7855,10 +7861,16 @@ for (v = var_table; v < var_table + var_table_size; v++)
   if (v->type == vtype_stringptr)
     assert_variable_notin(US v->name, *(USS v->value), &e);
 
+/* check dns and address trees */
+tree_walk(tree_dns_fails,     assert_variable_notin, &e);
+tree_walk(tree_duplicates,    assert_variable_notin, &e);
+tree_walk(tree_nonrecipients, assert_variable_notin, &e);
+tree_walk(tree_unusable,      assert_variable_notin, &e);
+
 if (e.var_name)
   log_write(0, LOG_MAIN|LOG_PANIC_DIE,
     "live variable '%s' destroyed by reset_store at %s:%d\n- value '%.64s'",
-    e.var_name, e.filename, e.linenumber, e.var_data);
+    e.var_name, filename, linenumber, e.var_data);
 }
 
 
diff --git a/src/src/queue.c b/src/src/queue.c
index f94681197..09a149e9a 100644
--- a/src/src/queue.c
+++ b/src/src/queue.c
@@ -699,6 +699,7 @@ for (i = queue_run_in_order ? -1 : 0;
       }
     }                                  /* End loop for list of messages */
 
+  tree_nonrecipients = NULL;
   store_reset(reset_point1);           /* Scavenge list of messages */
 
   /* If this was the first time through for random order processing, and
diff --git a/test/confs/4500 b/test/confs/4500
index f2e44beff..b53dff5b7 100644
--- a/test/confs/4500
+++ b/test/confs/4500
@@ -18,6 +18,9 @@ queue_run_in_order
 begin acl
 
 check_dkim:
+.ifdef BAD
+  warn	logwrite =	${lookup dnsdb{defer_never,txt=_adsp._domainkey.$dkim_cur_signer}{$value}{unknown}}
+.endif
 .ifdef OPTION
   warn	condition =	${if eq {$dkim_algo}{rsa-sha1}}
 	condition =	${if eq {$dkim_verify_status}{pass}}
diff --git a/test/log/4506 b/test/log/4506
index 07b3ee8ce..995dbde98 100644
--- a/test/log/4506
+++ b/test/log/4506
@@ -16,3 +16,8 @@
 1999-03-02 09:44:33 10HmbB-0005vi-00 signer: test.ex bits: 512
 1999-03-02 09:44:33 10HmbB-0005vi-00 DKIM: d=test.ex s=ses_sha256 c=simple/simple a=rsa-sha1 b=512 [verification failed - unspecified reason]
 1999-03-02 09:44:33 10HmbB-0005vi-00 <= CALLER@bloggs.com H=(xxx) [127.0.0.1] P=smtp S=sss id=qwerty1234@disco-zombie.net
+1999-03-02 09:44:33 exim x.yz daemon started: pid=pppp, no queue runs, listening for SMTP on port 1225
+1999-03-02 09:44:33 10HmbC-0005vi-00 unknown
+1999-03-02 09:44:33 10HmbC-0005vi-00 signer: test.ex bits: 0
+1999-03-02 09:44:33 10HmbC-0005vi-00 DKIM: d=test.ex s=sel c=simple/simple a=rsa-sha1 b=0 [invalid - signature tag missing or invalid]
+1999-03-02 09:44:33 10HmbC-0005vi-00 <= CALLER@bloggs.com H=(xxx) [127.0.0.1] P=smtp S=sss id=qwerty1234@disco-zombie.net
diff --git a/test/scripts/4500-DKIM/4506 b/test/scripts/4500-DKIM/4506
index e8d7c41f0..4499315d2 100644
--- a/test/scripts/4500-DKIM/4506
+++ b/test/scripts/4500-DKIM/4506
@@ -167,6 +167,40 @@ This is a simple test.
 QUIT
 ??? 221
 ****
+killdaemon
+#
+#
+# See what happens when we do a DNS lookup from the DKIM ACL
+exim -DSERVER=server -DBAD=bad -bd -oX PORT_D
+****
+# This should fail verify (missing header hash in sig header)
+#  - sha1, 1024b
+# Mail original in aux-fixed/4500.msg1.txt
+# Sig generated by: perl aux-fixed/dkim/sign.pl --method=simple/simple < aux-fixed/4500.msg1.txt
+client 127.0.0.1 PORT_D
+??? 220
+HELO xxx
+??? 250
+MAIL FROM:<CALLER@bloggs.com>
+??? 250
+RCPT TO:<a@test.ex>
+??? 250
+DATA
+??? 354
+DKIM-Signature: v=1; a=rsa-sha1; c=simple/simple; d=test.ex; h=from:to
+	:date:message-id:subject; s=sel; bh=OB9dZVu7+5/ufs3TH9leIcEpXSo=;
+From: mrgus@text.ex
+To: bakawolf@yahoo.com
+Date: Thu, 19 Nov 2015 17:00:07 -0700
+Message-ID: <qwerty1234@disco-zombie.net>
+Subject: simple test
+
+This is a simple test.
+.
+??? 250
+QUIT
+??? 221
+****
 #
 #
 killdaemon