Fix segfault on bad cmdline -f (sender) argument. Bug 2541

10 files changed, 40 insertions, 34 deletions

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 4875289b3..74568ce3f 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -149,6 +149,9 @@ JH/30 When an pipelined-connect fails at the first response, assume incorrect
 
 JH/31 Fix spurious detection of timeout while writing to transport filter.
 
+JH/32 Bug 2541: Fix segfault on bad cmdline -f (sender) argument.  Previously
+      an attempt to copy the string was made before checking it.
+
 
 Exim version 4.93
 -----------------
diff --git a/src/src/exim.c b/src/src/exim.c
index 5e2f43799..695618fd7 100644
--- a/src/src/exim.c
+++ b/src/src/exim.c
@@ -833,7 +833,7 @@ int start, end, domain;
 uschar *parse_error = NULL;
 uschar *address = parse_extract_address(s, &parse_error, &start, &end, &domain,
   FALSE);
-if (address == NULL)
+if (!address)
   {
   fprintf(stdout, "syntax error: %s\n", parse_error);
   *exit_value = 2;
@@ -2628,8 +2628,10 @@ for (i = 1; i < argc; i++)
 #ifdef SUPPORT_I18N
 	allow_utf8_domains = TRUE;
 #endif
-        sender_address = parse_extract_address(argrest, &errmess,
-          &dummy_start, &dummy_end, &sender_address_domain, TRUE);
+        if (!(sender_address = parse_extract_address(argrest, &errmess,
+		  &dummy_start, &dummy_end, &sender_address_domain, TRUE)))
+          exim_fail("exim: bad -f address \"%s\": %s\n", argrest, errmess);
+
 	sender_address = string_copy_taint(sender_address, TRUE);
 #ifdef SUPPORT_I18N
 	message_smtputf8 =  string_is_utf8(sender_address);
@@ -2637,8 +2639,6 @@ for (i = 1; i < argc; i++)
 #endif
         allow_domain_literals = FALSE;
         strip_trailing_dot = FALSE;
-        if (!sender_address)
-          exim_fail("exim: bad -f address \"%s\": %s\n", argrest, errmess);
         }
       f.sender_address_forced = TRUE;
       }
@@ -5474,8 +5474,7 @@ while (more)
           errmess = US"unqualified recipient address not allowed";
           }
 
-        if (recipient == NULL)
-          {
+        if (!recipient)
           if (error_handling == ERRORS_STDERR)
             {
             fprintf(stderr, "exim: bad recipient address \"%s\": %s\n",
@@ -5492,7 +5491,6 @@ while (more)
               moan_to_sender(ERRMESS_BADARGADDRESS, &eblock, NULL, stdin, TRUE)?
                 errors_sender_rc : EXIT_FAILURE;
             }
-          }
 
         receive_add_recipient(string_copy_taint(recipient, TRUE), -1);
         s = ss;
diff --git a/src/src/filter.c b/src/src/filter.c
index 98b6bc3e8..90e83e6b0 100644
--- a/src/src/filter.c
+++ b/src/src/filter.c
@@ -1510,7 +1510,7 @@ switch (c->type)
       parse_extract_address(pp, &error, &start, &end, &domain, FALSE);
     *p = saveend;
 
-    if (filter_thisaddress != NULL)
+    if (filter_thisaddress)
       {
       if ((filter_test != FTEST_NONE && debug_selector != 0) ||
           (debug_selector & D_filter) != 0)
@@ -1747,11 +1747,11 @@ while (commands)
 	  uschar *error;
 	  uschar *ss = parse_extract_address(s, &error, &start, &end, &domain,
 	    FALSE);
-	  if (ss != NULL)
-	    expargs[i] = ((filter_options & RDO_REWRITE) != 0)?
-	      rewrite_address(ss, TRUE, FALSE, global_rewrite_rules,
-		rewrite_existflags) :
-	      rewrite_address_qualify(ss, TRUE);
+	  if (ss)
+	    expargs[i] = filter_options & RDO_REWRITE
+	      ? rewrite_address(ss, TRUE, FALSE, global_rewrite_rules,
+				rewrite_existflags)
+	      : rewrite_address_qualify(ss, TRUE);
 	  else
 	    {
 	    *error_pointer = string_sprintf("malformed address \"%s\" in "
diff --git a/src/src/header.c b/src/src/header.c
index a6c44fac8..cbfc4f847 100644
--- a/src/src/header.c
+++ b/src/src/header.c
@@ -412,14 +412,13 @@ for (header_line * h = header_list; !yield && h; h = h->next)
       /* If there is some kind of syntax error, just give up on this header
       line. */
 
-      if (next == NULL) break;
+      if (!next) break;
 
       /* Otherwise, test for the pattern; a non-regex must be an exact match */
 
-      yield = (re == NULL)?
-        (strcmpic(next, pattern) == 0)
-        :
-        (pcre_exec(re, NULL, CS next, Ustrlen(next), 0, PCRE_EOPT, NULL, 0)
+      yield = !re
+        ? (strcmpic(next, pattern) == 0)
+        : (pcre_exec(re, NULL, CS next, Ustrlen(next), 0, PCRE_EOPT, NULL, 0)
           >= 0);
       }
     }
diff --git a/src/src/parse.c b/src/src/parse.c
index 71f48f379..5d50d6862 100644
--- a/src/src/parse.c
+++ b/src/src/parse.c
@@ -1614,14 +1614,14 @@ for (;;)
       {
       recipient =
         parse_extract_address(s+1, error, &start, &end, &domain, FALSE);
-      if (recipient != NULL)
-        recipient = (domain != 0)? NULL :
+      if (recipient)
+        recipient = domain != 0 ? NULL :
           string_sprintf("%s@%s", recipient, incoming_domain);
       }
 
     /* Try parsing the item as an address. */
 
-    if (recipient == NULL) recipient =
+    if (!recipient) recipient =
       parse_extract_address(s, error, &start, &end, &domain, FALSE);
 
     /* If item starts with / or | and is not a valid address, or there
@@ -2127,7 +2127,9 @@ while (Ufgets(buffer, sizeof(buffer), stdin) != NULL)
   buffer[Ustrlen(buffer) - 1] = 0;
   if (buffer[0] == 0) break;
   out = parse_extract_address(buffer, &errmess, &start, &end, &domain, FALSE);
-  if (out == NULL) printf("*** bad address: %s\n", errmess); else
+  if (!out)
+    printf("*** bad address: %s\n", errmess);
+  else
     {
     uschar extract[1024];
     Ustrncpy(extract, buffer+start, end-start);
@@ -2146,7 +2148,9 @@ while (Ufgets(buffer, sizeof(buffer), stdin) != NULL)
   buffer[Ustrlen(buffer) - 1] = 0;
   if (buffer[0] == 0) break;
   out = parse_extract_address(buffer, &errmess, &start, &end, &domain, FALSE);
-  if (out == NULL) printf("*** bad address: %s\n", errmess); else
+  if (!out)
+    printf("*** bad address: %s\n", errmess);
+  else
     {
     uschar extract[1024];
     Ustrncpy(extract, buffer+start, end-start);
@@ -2167,7 +2171,7 @@ while (Ufgets(buffer, sizeof(buffer), stdin) != NULL)
   buffer[Ustrlen(buffer) - 1] = 0;
   if (buffer[0] == 0) break;
   s = buffer;
-  while (*s != 0)
+  while (*s)
     {
     uschar *ss = parse_find_address_end(s, FALSE);
     int terminator = *ss;
@@ -2175,7 +2179,9 @@ while (Ufgets(buffer, sizeof(buffer), stdin) != NULL)
     out = parse_extract_address(buffer, &errmess, &start, &end, &domain, FALSE);
     *ss = terminator;
 
-    if (out == NULL) printf("*** bad address: %s\n", errmess); else
+    if (!out)
+      printf("*** bad address: %s\n", errmess);
+    else
       {
       uschar extract[1024];
       Ustrncpy(extract, buffer+start, end-start);
diff --git a/src/src/queue.c b/src/src/queue.c
index c9ac84be6..303a78086 100644
--- a/src/src/queue.c
+++ b/src/src/queue.c
@@ -1412,13 +1412,13 @@ switch(action)
       parse_extract_address(argv[recipients_arg], &errmess, &start, &end,
         &domain, (action == MSG_EDIT_SENDER));
 
-    if (recipient == NULL)
+    if (!recipient)
       {
       yield = FALSE;
       printf("- error while %s:\n  bad address %s: %s\n",
         doing, argv[recipients_arg], errmess);
       }
-    else if (recipient[0] != 0 && domain == 0)
+    else if (*recipient && domain == 0)
       {
       yield = FALSE;
       printf("- error while %s:\n  bad address %s: "
diff --git a/src/src/receive.c b/src/src/receive.c
index 5e8b6fbba..2745df6d1 100644
--- a/src/src/receive.c
+++ b/src/src/receive.c
@@ -2568,7 +2568,7 @@ if (extract_recip)
 
         If there are no recipients at all, an error will occur later. */
 
-        if (recipient == NULL && Ustrcmp(errmess, "empty address") != 0)
+        if (!recipient && Ustrcmp(errmess, "empty address") != 0)
           {
           int len = Ustrlen(s);
           error_block *b = store_get(sizeof(error_block), FALSE);
diff --git a/src/src/sieve.c b/src/src/sieve.c
index 5e8d1e6f4..286be789a 100644
--- a/src/src/sieve.c
+++ b/src/src/sieve.c
@@ -328,7 +328,7 @@ if (address->length>0)
   {
   ss = parse_extract_address(address->character, &error, &start, &end, &domain,
     FALSE);
-  if (ss == NULL)
+  if (!ss)
     {
     filter->errmsg=string_sprintf("malformed address \"%s\" (%s)",
       address->character, error);
diff --git a/src/src/transports/autoreply.c b/src/src/transports/autoreply.c
index 4b5ef8e17..e75349ed9 100644
--- a/src/src/transports/autoreply.c
+++ b/src/src/transports/autoreply.c
@@ -202,7 +202,7 @@ while (*s != 0)
   /* If there is some kind of syntax error, just give up on this header
   line. */
 
-  if (next == NULL) break;
+  if (!next) break;
 
   /* See if the address is on the never_mail list */
 
diff --git a/src/src/verify.c b/src/src/verify.c
index deca5bc6c..7b9d006f6 100644
--- a/src/src/verify.c
+++ b/src/src/verify.c
@@ -2294,7 +2294,7 @@ for (header_line * h = header_list; h && yield == OK; h = h->next)
         {
         if (!f.allow_unqualified_recipient) recipient = NULL;
         }
-      if (recipient == NULL) errmess = US"unqualified address not permitted";
+      if (!recipient) errmess = US"unqualified address not permitted";
       }
 
     /* It's an error if no address could be extracted, except for the special
@@ -2608,7 +2608,7 @@ for (int i = 0; i < 3 && !done; i++)
         /* If we found an empty address, just carry on with the next one, but
         kill the message. */
 
-        if (address == NULL && Ustrcmp(*log_msgptr, "empty address") == 0)
+        if (!address && Ustrcmp(*log_msgptr, "empty address") == 0)
           {
           *log_msgptr = NULL;
           s = ss;
@@ -2619,7 +2619,7 @@ for (int i = 0; i < 3 && !done; i++)
         function, and ensure that the failing address gets added to the error
         message. */
 
-        if (address == NULL)
+        if (!address)
           {
           new_ok = FAIL;
           while (ss > s && isspace(ss[-1])) ss--;