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

Broken-by: cc55f4208e

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 7ec669b1c013295b19ef788c74dd8dc55d253488..7d1d526d7e071c9dfbc2b978507422a53882a45e 100644 (file)
--- 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.
 
       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
 -----------------
 
 Exim version 4.90
 -----------------

diff --git a/src/src/expand.c b/src/src/expand.c
index e754fbc8cd3c8d25531153754cf8ed66032539bb..3a63a7c7827cfcc2e09186e35533d01b31705189 100644 (file)
--- 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 (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;
 }
 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
 */
 
           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                   *
 *************************************************/
 /*************************************************
 *              Expand and copy                   *
 *************************************************/


@@ -7812,8 +7819,6 @@ return (  (  Ustrstr(s, "failed to expand") != NULL
 * Error-checking for testsuite                   *
 *************************************************/
 typedef struct {
 * Error-checking for testsuite                   *
 *************************************************/
 typedef struct {

-  const char * filename;
-  int          linenumber;

   uschar *     region_start;
   uschar *     region_end;
   const uschar *var_name;
   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)
 {
 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;
 
 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);
 
   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'",
 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 f9468119780b603b91382a4461238bc2169fc77a..09a149e9a860c356cbc3bd660a3250efebe12498 100644 (file)
--- 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 */
 
       }
     }                                  /* 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
   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 f2e44beff244d7b059a8d65705154144b55049c9..b53dff5b7f2b9b4d4855f82f8485050b0078e0df 100644 (file)
--- a/test/confs/4500
+++ b/test/confs/4500
@@ -18,6 +18,9 @@ queue_run_in_order
 begin acl
 
 check_dkim:
 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}}
 .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 07b3ee8ce8d3340840f537c438caf40d973b0abf..995dbde98085a15f4736db9eb4263a5dfcdcbc7a 100644 (file)
--- 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 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 e8d7c41f021fc82182cfc640b3caf432a42a4c32..4499315d27ecf186c24d8a376fcc0985b871f717 100644 (file)
--- a/test/scripts/4500-DKIM/4506
+++ b/test/scripts/4500-DKIM/4506
@@ -161,6 +161,40 @@ Date: Thu, 19 Nov 2015 17:00:07 -0700
 Message-ID: <qwerty1234@disco-zombie.net>
 Subject: simple test
 
 Message-ID: <qwerty1234@disco-zombie.net>
 Subject: simple test
 

+This is a simple test.
+.
+??? 250
+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
 This is a simple test.
 .
 ??? 250