Avoid wait-for-tick on single-message connections
[exim.git] / src / src / exim.c
index 3e08f0376fbe2019738ddd4c1ac46167030999ce..e15d5e4760d252557d471c5e4e53b645e1f45b19 100644 (file)
@@ -211,6 +211,19 @@ exit(1);
 }
 
 
+/***********************************************
+*            Handler for SIGSEGV               *
+***********************************************/
+
+static void
+segv_handler(int sig)
+{
+log_write(0, LOG_MAIN|LOG_PANIC, "SIGSEGV (maybe attempt to write to immutable memory)");
+signal(SIGSEGV, SIG_DFL);
+kill(getpid(), sig);
+}
+
+
 /*************************************************
 *             Handler for SIGUSR1                *
 *************************************************/
@@ -233,18 +246,8 @@ int fd;
 
 os_restarting_signal(sig, usr1_handler);
 
-if ((fd = Uopen(process_log_path, O_APPEND|O_WRONLY, LOG_MODE)) < 0)
-  {
-  /* If we are already running as the Exim user, try to create it in the
-  current process (assuming spool_directory exists). Otherwise, if we are
-  root, do the creation in an exim:exim subprocess. */
-
-  int euid = geteuid();
-  if (euid == exim_uid)
-    fd = Uopen(process_log_path, O_CREAT|O_APPEND|O_WRONLY, LOG_MODE);
-  else if (euid == root_uid)
-    fd = log_create_as_exim(process_log_path);
-  }
+if (!process_log_path) return;
+fd = log_open_as_exim(process_log_path);
 
 /* If we are neither exim nor root, or if we failed to create the log file,
 give up. There is not much useful we can do with errors, since we don't want
@@ -441,9 +444,10 @@ function prepares for the time when things are faster - and it also copes with
 clocks that go backwards.
 
 Arguments:
-  tgt_tv       A timeval which was used to create uniqueness; its usec field
+  prev_tv      A timeval which was used to create uniqueness; its usec field
                  has been rounded down to the value of the resolution.
                  We want to be sure the current time is greater than this.
+                On return, updated to current (rounded down).
   resolution   The resolution that was used to divide the microseconds
                  (1 for maildir, larger for message ids)
 
@@ -451,7 +455,7 @@ Returns:       nothing
 */
 
 void
-exim_wait_tick(struct timeval * tgt_tv, int resolution)
+exim_wait_tick(struct timeval * prev_tv, int resolution)
 {
 struct timeval now_tv;
 long int now_true_usec;
@@ -460,13 +464,13 @@ exim_gettime(&now_tv);
 now_true_usec = now_tv.tv_usec;
 now_tv.tv_usec = (now_true_usec/resolution) * resolution;
 
-while (exim_tvcmp(&now_tv, tgt_tv) <= 0)
+while (exim_tvcmp(&now_tv, prev_tv) <= 0)
   {
   struct itimerval itval;
   itval.it_interval.tv_sec = 0;
   itval.it_interval.tv_usec = 0;
-  itval.it_value.tv_sec = tgt_tv->tv_sec - now_tv.tv_sec;
-  itval.it_value.tv_usec = tgt_tv->tv_usec + resolution - now_true_usec;
+  itval.it_value.tv_sec = prev_tv->tv_sec - now_tv.tv_sec;
+  itval.it_value.tv_usec = prev_tv->tv_usec + resolution - now_true_usec;
 
   /* We know that, overall, "now" is less than or equal to "then". Therefore, a
   negative value for the microseconds is possible only in the case when "now"
@@ -484,7 +488,7 @@ while (exim_tvcmp(&now_tv, tgt_tv) <= 0)
     if (!f.running_in_test_harness)
       {
       debug_printf("tick check: " TIME_T_FMT ".%06lu " TIME_T_FMT ".%06lu\n",
-        tgt_tv->tv_sec, (long) tgt_tv->tv_usec,
+        prev_tv->tv_sec, (long) prev_tv->tv_usec,
                now_tv.tv_sec, (long) now_tv.tv_usec);
       debug_printf("waiting " TIME_T_FMT ".%06lu sec\n",
         itval.it_value.tv_sec, (long) itval.it_value.tv_usec);
@@ -500,6 +504,7 @@ while (exim_tvcmp(&now_tv, tgt_tv) <= 0)
   now_true_usec = now_tv.tv_usec;
   now_tv.tv_usec = (now_true_usec/resolution) * resolution;
   }
+*prev_tv = now_tv;
 }
 
 
@@ -760,6 +765,25 @@ vfprintf(stderr, fmt, ap);
 exit(EXIT_FAILURE);
 }
 
+/* fail if a length is too long */
+static inline void
+exim_len_fail_toolong(int itemlen, int maxlen, const char *description)
+{
+if (itemlen <= maxlen)
+  return;
+fprintf(stderr, "exim: length limit exceeded (%d > %d) for: %s\n",
+        itemlen, maxlen, description);
+exit(EXIT_FAILURE);
+}
+
+/* only pass through the string item back to the caller if it's short enough */
+static inline const uschar *
+exim_str_fail_toolong(const uschar *item, int maxlen, const char *description)
+{
+exim_len_fail_toolong(Ustrlen(item), maxlen, description);
+return item;
+}
+
 /* exim_chown_failure() called from exim_chown()/exim_fchown() on failure
 of chown()/fchown().  See src/functions.h for more explanation */
 int
@@ -1540,10 +1564,11 @@ Arguments:
 */
 
 static void
-expansion_test_line(uschar * line)
+expansion_test_line(const uschar * line)
 {
 int len;
 BOOL dummy_macexp;
+uschar * s;
 
 Ustrncpy(big_buffer, line, big_buffer_size);
 big_buffer[big_buffer_size-1] = '\0';
@@ -1557,7 +1582,7 @@ if (isupper(big_buffer[0]))
     printf("Defined macro '%s'\n", mlast->name);
   }
 else
-  if ((line = expand_string(big_buffer))) printf("%s\n", CS line);
+  if ((s = expand_string(big_buffer))) printf("%s\n", CS s);
   else printf("Failed: %s\n", expand_string_message);
 }
 
@@ -1621,7 +1646,6 @@ BOOL list_queue = FALSE;
 BOOL list_options = FALSE;
 BOOL list_config = FALSE;
 BOOL local_queue_only;
-BOOL more = TRUE;
 BOOL one_msg_action = FALSE;
 BOOL opt_D_used = FALSE;
 BOOL queue_only_set = FALSE;
@@ -1641,10 +1665,10 @@ uschar *cmdline_syslog_name = NULL;
 uschar *start_queue_run_id = NULL;
 uschar *stop_queue_run_id = NULL;
 uschar *expansion_test_message = NULL;
-uschar *ftest_domain = NULL;
-uschar *ftest_localpart = NULL;
-uschar *ftest_prefix = NULL;
-uschar *ftest_suffix = NULL;
+const uschar *ftest_domain = NULL;
+const uschar *ftest_localpart = NULL;
+const uschar *ftest_prefix = NULL;
+const uschar *ftest_suffix = NULL;
 uschar *log_oneline = NULL;
 uschar *malware_test_file = NULL;
 uschar *real_sender_address;
@@ -1739,6 +1763,9 @@ f.running_in_test_harness =
 if (f.running_in_test_harness)
   debug_store = TRUE;
 
+/* Protect against abusive argv[0] */
+exim_str_fail_toolong(argv[0], PATH_MAX, "argv[0]");
+
 /* The C standard says that the equivalent of setlocale(LC_ALL, "C") is obeyed
 at the start of a program; however, it seems that some environments do not
 follow this. A "strange" locale can affect the formatting of timestamps, so we
@@ -1792,7 +1819,8 @@ descriptive text. */
 
 process_info = store_get(PROCESS_INFO_SIZE, TRUE);     /* tainted */
 set_process_info("initializing");
-os_restarting_signal(SIGUSR1, usr1_handler);
+os_restarting_signal(SIGUSR1, usr1_handler);           /* exiwhat */
+signal(SIGSEGV, segv_handler);                         /* log faults */
 
 /* If running in a dockerized environment, the TERM signal is only
 delegated to the PID 1 if we request it by setting an signal handler */
@@ -2140,10 +2168,10 @@ on the second character (the one after '-'), to save some effort. */
            {
            if (++i >= argc)
              exim_fail("exim: string expected after %s\n", arg);
-           if (Ustrcmp(argrest, "d") == 0) ftest_domain = argv[i];
-           else if (Ustrcmp(argrest, "l") == 0) ftest_localpart = argv[i];
-           else if (Ustrcmp(argrest, "p") == 0) ftest_prefix = argv[i];
-           else if (Ustrcmp(argrest, "s") == 0) ftest_suffix = argv[i];
+           if (Ustrcmp(argrest, "d") == 0) ftest_domain = exim_str_fail_toolong(argv[i], EXIM_DOMAINNAME_MAX, "-bfd");
+           else if (Ustrcmp(argrest, "l") == 0) ftest_localpart = exim_str_fail_toolong(argv[i], EXIM_LOCALPART_MAX, "-bfl");
+           else if (Ustrcmp(argrest, "p") == 0) ftest_prefix = exim_str_fail_toolong(argv[i], EXIM_LOCALPART_MAX, "-bfp");
+           else if (Ustrcmp(argrest, "s") == 0) ftest_suffix = exim_str_fail_toolong(argv[i], EXIM_LOCALPART_MAX, "-bfs");
            else badarg = TRUE;
            }
          break;
@@ -2153,7 +2181,7 @@ on the second character (the one after '-'), to save some effort. */
          if (!*argrest || Ustrcmp(argrest, "c") == 0)
            {
            if (++i >= argc) { badarg = TRUE; break; }
-           sender_host_address = string_copy_taint(argv[i], TRUE);
+           sender_host_address = string_copy_taint(exim_str_fail_toolong(argv[i], EXIM_IPADDR_MAX, "-bh"), TRUE);
            host_checking = checking = f.log_testing_mode = TRUE;
            f.host_checking_callout = *argrest == 'c';
            message_logs = FALSE;
@@ -2611,7 +2639,7 @@ on the second character (the one after '-'), to save some effort. */
     case 'F':
     if (!*argrest)
       if (++i < argc) argrest = argv[i]; else { badarg = TRUE; break; }
-    originator_name = string_copy_taint(argrest, TRUE);
+    originator_name = string_copy_taint(exim_str_fail_toolong(argrest, EXIM_HUMANNAME_MAX, "-F"), TRUE);
     f.sender_name_forced = TRUE;
     break;
 
@@ -2637,6 +2665,7 @@ on the second character (the one after '-'), to save some effort. */
       uschar *errmess;
       if (!*argrest)
         if (i+1 < argc) argrest = argv[++i]; else { badarg = TRUE; break; }
+      (void) exim_str_fail_toolong(argrest, EXIM_DISPLAYMAIL_MAX, "-f");
       if (!*argrest)
         *(sender_address = store_get(1, FALSE)) = '\0';  /* Ensure writeable memory */
       else
@@ -2733,9 +2762,9 @@ on the second character (the one after '-'), to save some effort. */
       if (msg_action_arg >= 0)
         exim_fail("exim: incompatible arguments\n");
 
-      continue_transport = string_copy_taint(argv[++i], TRUE);
-      continue_hostname = string_copy_taint(argv[++i], TRUE);
-      continue_host_address = string_copy_taint(argv[++i], TRUE);
+      continue_transport = string_copy_taint(exim_str_fail_toolong(argv[++i], EXIM_DRIVERNAME_MAX, "-C internal transport"), TRUE);
+      continue_hostname = string_copy_taint(exim_str_fail_toolong(argv[++i], EXIM_HOSTNAME_MAX, "-C internal hostname"), TRUE);
+      continue_host_address = string_copy_taint(exim_str_fail_toolong(argv[++i], EXIM_IPADDR_MAX, "-C internal hostaddr"), TRUE);
       continue_sequence = Uatoi(argv[++i]);
       msg_action = MSG_DELIVER;
       msg_action_arg = ++i;
@@ -2780,13 +2809,15 @@ on the second character (the one after '-'), to save some effort. */
     /* -MCd: for debug, set a process-purpose string */
 
        case 'd': if (++i < argc)
-                   process_purpose = string_copy_taint(argv[i], TRUE);
+                   process_purpose = string_copy_taint(exim_str_fail_toolong(argv[i], EXIM_DRIVERNAME_MAX, "-MCd"), TRUE);
                  else badarg = TRUE;
                  break;
 
-    /* -MCG: set the queue name, to a non-default value */
+    /* -MCG: set the queue name, to a non-default value. Arguably, anything
+       from the commandline should be tainted - but we will need an untainted
+       value for the spoolfile when doing a -odi delivery process. */
 
-       case 'G': if (++i < argc) queue_name = string_copy_taint(argv[i], TRUE);
+       case 'G': if (++i < argc) queue_name = string_copy_taint(exim_str_fail_toolong(argv[i], EXIM_DRIVERNAME_MAX, "-MCG"), FALSE);
                  else badarg = TRUE;
                  break;
 
@@ -2859,7 +2890,7 @@ on the second character (the one after '-'), to save some effort. */
        case 'r':
        case 's': if (++i < argc)
                    {
-                   continue_proxy_sni = string_copy_taint(argv[i], TRUE);
+                   continue_proxy_sni = string_copy_taint(exim_str_fail_toolong(argv[i], EXIM_HOSTNAME_MAX, "-MCr/-MCs"), TRUE);
                    if (argrest[1] == 'r') continue_proxy_dane = TRUE;
                    }
                  else badarg = TRUE;
@@ -2871,13 +2902,13 @@ on the second character (the one after '-'), to save some effort. */
     and the TLS cipher. */
 
        case 't': if (++i < argc)
-                   sending_ip_address = string_copy_taint(argv[i], TRUE);
+                   sending_ip_address = string_copy_taint(exim_str_fail_toolong(argv[i], EXIM_IPADDR_MAX, "-MCt IP"), TRUE);
                  else badarg = TRUE;
                  if (++i < argc)
                    sending_port = (int)(Uatol(argv[i]));
                  else badarg = TRUE;
                  if (++i < argc)
-                   continue_proxy_cipher = string_copy_taint(argv[i], TRUE);
+                   continue_proxy_cipher = string_copy_taint(exim_str_fail_toolong(argv[i], EXIM_CIPHERNAME_MAX, "-MCt cipher"), TRUE);
                  else badarg = TRUE;
                  /*FALLTHROUGH*/
 
@@ -2904,6 +2935,7 @@ on the second character (the one after '-'), to save some effort. */
     following options which are followed by a single message id, and which
     act on that message. Some of them use the "recipient" addresses as well.
        -Mar  add recipient(s)
+       -MG   move to a different queue
        -Mmad mark all recipients delivered
        -Mmd  mark recipients(s) delivered
        -Mes  edit sender
@@ -2939,7 +2971,7 @@ on the second character (the one after '-'), to save some effort. */
    else if (Ustrcmp(argrest, "G") == 0)
       {
       msg_action = MSG_SETQUEUE;
-      queue_name_dest = string_copy_taint(argv[++i], TRUE);
+      queue_name_dest = string_copy_taint(exim_str_fail_toolong(argv[++i], EXIM_DRIVERNAME_MAX, "-MG"), TRUE);
       }
     else if (Ustrcmp(argrest, "mad") == 0)
       {
@@ -3152,27 +3184,27 @@ on the second character (the one after '-'), to save some effort. */
        /* -oMa: Set sender host address */
 
        if (Ustrcmp(argrest, "a") == 0)
-         sender_host_address = string_copy_taint(argv[++i], TRUE);
+         sender_host_address = string_copy_taint(exim_str_fail_toolong(argv[++i], EXIM_IPADDR_MAX, "-oMa"), TRUE);
 
        /* -oMaa: Set authenticator name */
 
        else if (Ustrcmp(argrest, "aa") == 0)
-         sender_host_authenticated = string_copy_taint(argv[++i], TRUE);
+         sender_host_authenticated = string_copy_taint(exim_str_fail_toolong(argv[++i], EXIM_DRIVERNAME_MAX, "-oMaa"), TRUE);
 
        /* -oMas: setting authenticated sender */
 
        else if (Ustrcmp(argrest, "as") == 0)
-         authenticated_sender = string_copy_taint(argv[++i], TRUE);
+         authenticated_sender = string_copy_taint(exim_str_fail_toolong(argv[++i], EXIM_EMAILADDR_MAX, "-oMas"), TRUE);
 
        /* -oMai: setting authenticated id */
 
        else if (Ustrcmp(argrest, "ai") == 0)
-         authenticated_id = string_copy_taint(argv[++i], TRUE);
+         authenticated_id = string_copy_taint(exim_str_fail_toolong(argv[++i], EXIM_EMAILADDR_MAX, "-oMas"), TRUE);
 
        /* -oMi: Set incoming interface address */
 
        else if (Ustrcmp(argrest, "i") == 0)
-         interface_address = string_copy_taint(argv[++i], TRUE);
+         interface_address = string_copy_taint(exim_str_fail_toolong(argv[++i], EXIM_IPADDR_MAX, "-oMi"), TRUE);
 
        /* -oMm: Message reference */
 
@@ -3192,19 +3224,19 @@ on the second character (the one after '-'), to save some effort. */
          if (received_protocol)
            exim_fail("received_protocol is set already\n");
          else
-           received_protocol = string_copy_taint(argv[++i], TRUE);
+           received_protocol = string_copy_taint(exim_str_fail_toolong(argv[++i], EXIM_DRIVERNAME_MAX, "-oMr"), TRUE);
 
        /* -oMs: Set sender host name */
 
        else if (Ustrcmp(argrest, "s") == 0)
-         sender_host_name = string_copy_taint(argv[++i], TRUE);
+         sender_host_name = string_copy_taint(exim_str_fail_toolong(argv[++i], EXIM_HOSTNAME_MAX, "-oMs"), TRUE);
 
        /* -oMt: Set sender ident */
 
        else if (Ustrcmp(argrest, "t") == 0)
          {
          sender_ident_set = TRUE;
-         sender_ident = string_copy_taint(argv[++i], TRUE);
+         sender_ident = string_copy_taint(exim_str_fail_toolong(argv[++i], EXIM_IDENTUSER_MAX, "-oMt"), TRUE);
          }
 
        /* Else a bad argument */
@@ -3229,6 +3261,10 @@ on the second character (the one after '-'), to save some effort. */
         -oPX:       delete pid file of daemon */
 
       case 'P':
+       if (!f.running_in_test_harness && real_uid != root_uid && real_uid != exim_uid)
+         exim_fail("exim: only uid=%d or uid=%d can use -oP and -oPX "
+                    "(uid=%d euid=%d | %d)\n",
+                    root_uid, exim_uid, getuid(), geteuid(), real_uid);
        if (!*argrest) override_pid_file_path = argv[++i];
        else if (Ustrcmp(argrest, "X") == 0) delete_pid_file();
        else badarg = TRUE;
@@ -3254,10 +3290,11 @@ on the second character (the one after '-'), to save some effort. */
        break;
 
       /* -oX <list>: Override local_interfaces and/or default daemon ports */
+      /* Limits: Is there a real limit we want here?  1024 is very arbitrary. */
 
       case 'X':
        if (*argrest) badarg = TRUE;
-       else override_local_interfaces = string_copy_taint(argv[++i], TRUE);
+       else override_local_interfaces = string_copy_taint(exim_str_fail_toolong(argv[++i], 1024, "-oX"), TRUE);
        break;
 
       /* -oY: Override creation of daemon notifier socket */
@@ -3305,9 +3342,10 @@ on the second character (the one after '-'), to save some effort. */
         exim_fail("received_protocol is set already\n");
 
       if (!hn)
-        received_protocol = string_copy_taint(argrest, TRUE);
+        received_protocol = string_copy_taint(exim_str_fail_toolong(argrest, EXIM_DRIVERNAME_MAX, "-p<protocol>"), TRUE);
       else
         {
+        (void) exim_str_fail_toolong(argrest, (EXIM_DRIVERNAME_MAX+1+EXIM_HOSTNAME_MAX), "-p<protocol>:<host>");
         received_protocol = string_copyn_taint(argrest, hn - argrest, TRUE);
         sender_host_name = string_copy_taint(hn + 1, TRUE);
         }
@@ -3363,6 +3401,7 @@ on the second character (the one after '-'), to save some effort. */
       {
       int i;
       for (argrest++, i = 0; argrest[i] && argrest[i] != '/'; ) i++;
+      exim_len_fail_toolong(i, EXIM_DRIVERNAME_MAX, "-q*G<name>");
       queue_name = string_copyn(argrest, i);
       argrest += i;
       if (*argrest == '/') argrest++;
@@ -3392,7 +3431,10 @@ on the second character (the one after '-'), to save some effort. */
 
 
     case 'R':   /* Synonymous with -qR... */
-    receiving_message = FALSE;
+      {
+      const uschar *tainted_selectstr;
+
+      receiving_message = FALSE;
 
     /* -Rf:   As -R (below) but force all deliveries,
        -Rff:  Ditto, but also thaw all frozen messages,
@@ -3403,35 +3445,41 @@ on the second character (the one after '-'), to save some effort. */
     in all cases provided there are no further characters in this
     argument. */
 
-    if (*argrest)
-      for (int i = 0; i < nelem(rsopts); i++)
-        if (Ustrcmp(argrest, rsopts[i]) == 0)
-          {
-          if (i != 2) f.queue_run_force = TRUE;
-          if (i >= 2) f.deliver_selectstring_regex = TRUE;
-          if (i == 1 || i == 4) f.deliver_force_thaw = TRUE;
-          argrest += Ustrlen(rsopts[i]);
-          }
+      if (*argrest)
+       for (int i = 0; i < nelem(rsopts); i++)
+         if (Ustrcmp(argrest, rsopts[i]) == 0)
+           {
+           if (i != 2) f.queue_run_force = TRUE;
+           if (i >= 2) f.deliver_selectstring_regex = TRUE;
+           if (i == 1 || i == 4) f.deliver_force_thaw = TRUE;
+           argrest += Ustrlen(rsopts[i]);
+           }
 
     /* -R: Set string to match in addresses for forced queue run to
     pick out particular messages. */
 
-    if (*argrest)
-      deliver_selectstring = string_copy_taint(argrest, TRUE);
-    else if (i+1 < argc)
-      deliver_selectstring = string_copy_taint(argv[++i], TRUE);
-    else
-      exim_fail("exim: string expected after -R\n");
+      /* Avoid attacks from people providing very long strings, and do so before
+      we make copies. */
+      if (*argrest)
+       tainted_selectstr = argrest;
+      else if (i+1 < argc)
+       tainted_selectstr = argv[++i];
+      else
+       exim_fail("exim: string expected after -R\n");
+      deliver_selectstring = string_copy_taint(exim_str_fail_toolong(tainted_selectstr, EXIM_EMAILADDR_MAX, "-R"), TRUE);
+      }
     break;
 
-
     /* -r: an obsolete synonym for -f (see above) */
 
 
     /* -S: Like -R but works on sender. */
 
     case 'S':   /* Synonymous with -qS... */
-    receiving_message = FALSE;
+      {
+      const uschar *tainted_selectstr;
+
+      receiving_message = FALSE;
 
     /* -Sf:   As -S (below) but force all deliveries,
        -Sff:  Ditto, but also thaw all frozen messages,
@@ -3442,25 +3490,27 @@ on the second character (the one after '-'), to save some effort. */
     in all cases provided there are no further characters in this
     argument. */
 
-    if (*argrest)
-      for (int i = 0; i < nelem(rsopts); i++)
-        if (Ustrcmp(argrest, rsopts[i]) == 0)
-          {
-          if (i != 2) f.queue_run_force = TRUE;
-          if (i >= 2) f.deliver_selectstring_sender_regex = TRUE;
-          if (i == 1 || i == 4) f.deliver_force_thaw = TRUE;
-          argrest += Ustrlen(rsopts[i]);
-          }
+      if (*argrest)
+       for (int i = 0; i < nelem(rsopts); i++)
+         if (Ustrcmp(argrest, rsopts[i]) == 0)
+           {
+           if (i != 2) f.queue_run_force = TRUE;
+           if (i >= 2) f.deliver_selectstring_sender_regex = TRUE;
+           if (i == 1 || i == 4) f.deliver_force_thaw = TRUE;
+           argrest += Ustrlen(rsopts[i]);
+           }
 
     /* -S: Set string to match in addresses for forced queue run to
     pick out particular messages. */
 
-    if (*argrest)
-      deliver_selectstring_sender = string_copy_taint(argrest, TRUE);
-    else if (i+1 < argc)
-      deliver_selectstring_sender = string_copy_taint(argv[++i], TRUE);
-    else
-      exim_fail("exim: string expected after -S\n");
+      if (*argrest)
+       tainted_selectstr = argrest;
+      else if (i+1 < argc)
+       tainted_selectstr = argv[++i];
+      else
+       exim_fail("exim: string expected after -S\n");
+      deliver_selectstring_sender = string_copy_taint(exim_str_fail_toolong(tainted_selectstr, EXIM_EMAILADDR_MAX, "-S"), TRUE);
+      }
     break;
 
     /* -Tqt is an option that is exclusively for use by the testing suite.
@@ -3542,10 +3592,12 @@ on the second character (the one after '-'), to save some effort. */
         exim_fail("exim: string expected after -X\n");
     break;
 
+    /* -z: a line of text to log */
+
     case 'z':
     if (!*argrest)
       if (++i < argc)
-       log_oneline = string_copy_taint(argv[i], TRUE);
+       log_oneline = string_copy_taint(exim_str_fail_toolong(argv[i], 2048, "-z logtext"), TRUE);
       else
         exim_fail("exim: file name expected after %s\n", argv[i-1]);
     break;
@@ -3651,7 +3703,7 @@ else
   {
   struct rlimit rlp;
 
-  #ifdef RLIMIT_NOFILE
+#ifdef RLIMIT_NOFILE
   if (getrlimit(RLIMIT_NOFILE, &rlp) < 0)
     {
     log_write(0, LOG_MAIN|LOG_PANIC, "getrlimit(RLIMIT_NOFILE) failed: %s",
@@ -3674,9 +3726,9 @@ else
           strerror(errno));
       }
     }
-  #endif
+#endif
 
-  #ifdef RLIMIT_NPROC
+#ifdef RLIMIT_NPROC
   if (getrlimit(RLIMIT_NPROC, &rlp) < 0)
     {
     log_write(0, LOG_MAIN|LOG_PANIC, "getrlimit(RLIMIT_NPROC) failed: %s",
@@ -3684,20 +3736,20 @@ else
     rlp.rlim_cur = rlp.rlim_max = 0;
     }
 
-  #ifdef RLIM_INFINITY
+ifdef RLIM_INFINITY
   if (rlp.rlim_cur != RLIM_INFINITY && rlp.rlim_cur < 1000)
     {
     rlp.rlim_cur = rlp.rlim_max = RLIM_INFINITY;
-  #else
+else
   if (rlp.rlim_cur < 1000)
     {
     rlp.rlim_cur = rlp.rlim_max = 1000;
-  #endif
+endif
     if (setrlimit(RLIMIT_NPROC, &rlp) < 0)
       log_write(0, LOG_MAIN|LOG_PANIC, "setrlimit(RLIMIT_NPROC) failed: %s",
         strerror(errno));
     }
-  #endif
+#endif
   }
 
 /* Exim is normally entered as root (but some special configurations are
@@ -3820,6 +3872,7 @@ is equivalent to the ability to modify a setuid binary!
 This needs to happen before we read the main configuration. */
 init_lookup_list();
 
+/*XXX this excrescence could move to the testsuite standard config setup file */
 #ifdef SUPPORT_I18N
 if (f.running_in_test_harness) smtputf8_advertise_hosts = NULL;
 #endif
@@ -3835,6 +3888,11 @@ during readconf_main() some expansion takes place already. */
 /* Store the initial cwd before we change directories.  Can be NULL if the
 dir has already been unlinked. */
 initial_cwd = os_getcwd(NULL, 0);
+if (!initial_cwd && errno)
+  exim_fail("exim: getting initial cwd failed: %s\n", strerror(errno));
+
+if (initial_cwd && (strlen(CCS initial_cwd) >= BIG_BUFFER_SIZE))
+  exim_fail("exim: initial cwd is far too long (%d)\n", Ustrlen(CCS initial_cwd));
 
 /* checking:
     -be[m] expansion test        -
@@ -3853,19 +3911,21 @@ issues (currently about tls_advertise_hosts and keep_environment not being
 defined) */
 
   {
+  int old_pool = store_pool;
 #ifdef MEASURE_TIMING
   struct timeval t0, diff;
   (void)gettimeofday(&t0, NULL);
 #endif
 
+  store_pool = POOL_CONFIG;
   readconf_main(checking || list_options);
+  store_pool = old_pool;
 
 #ifdef MEASURE_TIMING
   report_time_since(&t0, US"readconf_main (delta)");
 #endif
   }
 
-
 /* Now in directory "/" */
 
 if (cleanup_environment() == FALSE)
@@ -4122,11 +4182,9 @@ if (  (debug_selector & D_any  ||  LOGGING(arguments))
     p += 13;
   else
     {
-    Ustrncpy(p + 4, initial_cwd, big_buffer_size-5);
-    p += 4 + Ustrlen(initial_cwd);
-    /* in case p is near the end and we don't provide enough space for
-     * string_format to be willing to write. */
-    *p = '\0';
+    p += 4;
+    snprintf(CS p, big_buffer_size - (p - big_buffer), "%s", CCS initial_cwd);
+    p += Ustrlen(CCS p);
     }
 
   (void)string_format(p, big_buffer_size - (p - big_buffer), " %d args:", argc);
@@ -4454,7 +4512,13 @@ if (msg_action_arg > 0 && msg_action != MSG_DELIVER && msg_action != MSG_LOAD)
   event_action gets expanded */
 
   if (msg_action == MSG_REMOVE)
+    {
+    int old_pool = store_pool;
+    store_pool = POOL_CONFIG;
     readconf_rest();
+    store_pool = old_pool;
+    store_writeprotect(POOL_CONFIG);
+    }
 
   if (!one_msg_action)
     {
@@ -4479,12 +4543,16 @@ Now, since the intro of the ${acl } expansion, ACL definitions may be
 needed in transports so we lost the optimisation. */
 
   {
+  int old_pool = store_pool;
 #ifdef MEASURE_TIMING
   struct timeval t0, diff;
   (void)gettimeofday(&t0, NULL);
 #endif
 
+  store_pool = POOL_CONFIG;
   readconf_rest();
+  store_pool = old_pool;
+  store_writeprotect(POOL_CONFIG);
 
 #ifdef MEASURE_TIMING
   report_time_since(&t0, US"readconf_rest (delta)");
@@ -4513,14 +4581,14 @@ if (test_retry_arg >= 0)
   retry_config *yield;
   int basic_errno = 0;
   int more_errno = 0;
-  uschar *s1, *s2;
+  const uschar *s1, *s2;
 
   if (test_retry_arg >= argc)
     {
     printf("-brt needs a domain or address argument\n");
     exim_exit(EXIT_FAILURE);
     }
-  s1 = argv[test_retry_arg++];
+  s1 = exim_str_fail_toolong(argv[test_retry_arg++], EXIM_EMAILADDR_MAX, "-brt");
   s2 = NULL;
 
   /* If the first argument contains no @ and no . it might be a local user
@@ -4536,13 +4604,13 @@ if (test_retry_arg >= 0)
   /* There may be an optional second domain arg. */
 
   if (test_retry_arg < argc && Ustrchr(argv[test_retry_arg], '.') != NULL)
-    s2 = argv[test_retry_arg++];
+    s2 = exim_str_fail_toolong(argv[test_retry_arg++], EXIM_DOMAINNAME_MAX, "-brt 2nd");
 
   /* The final arg is an error name */
 
   if (test_retry_arg < argc)
     {
-    uschar *ss = argv[test_retry_arg];
+    const uschar *ss = exim_str_fail_toolong(argv[test_retry_arg], EXIM_DRIVERNAME_MAX, "-brt 3rd");
     uschar *error =
       readconf_retry_error(ss, ss + Ustrlen(ss), &basic_errno, &more_errno);
     if (error != NULL)
@@ -4644,11 +4712,11 @@ if (list_options)
         Ustrcmp(argv[i], "macro") == 0 ||
         Ustrcmp(argv[i], "environment") == 0))
       {
-      fail |= !readconf_print(argv[i+1], argv[i], flag_n);
+      fail |= !readconf_print(exim_str_fail_toolong(argv[i+1], EXIM_DRIVERNAME_MAX, "-bP name"), argv[i], flag_n);
       i++;
       }
     else
-      fail = !readconf_print(argv[i], NULL, flag_n);
+      fail = !readconf_print(exim_str_fail_toolong(argv[i], EXIM_DRIVERNAME_MAX, "-bP item"), NULL, flag_n);
     }
   exim_exit(fail ? EXIT_FAILURE : EXIT_SUCCESS);
   }
@@ -4693,6 +4761,7 @@ if (msg_action_arg > 0 && msg_action != MSG_LOAD)
     pid_t pid;
     /*XXX This use of argv[i] for msg_id should really be tainted, but doing
     that runs into a later copy into the untainted global message_id[] */
+    /*XXX Do we need a length limit check here? */
     if (i == argc - 1)
       (void)deliver_message(argv[i], forced_delivery, deliver_give_up);
     else if ((pid = exim_fork(US"cmdline-delivery")) == 0)
@@ -4898,7 +4967,7 @@ if (test_rewrite_arg >= 0)
     printf("-brw needs an address argument\n");
     exim_exit(EXIT_FAILURE);
     }
-  rewrite_test(argv[test_rewrite_arg]);
+  rewrite_test(exim_str_fail_toolong(argv[test_rewrite_arg], EXIM_EMAILADDR_MAX, "-brw"));
   exim_exit(EXIT_SUCCESS);
   }
 
@@ -4991,7 +5060,7 @@ if (verify_address_mode || f.address_test_mode)
     while (recipients_arg < argc)
       {
       /* Supplied addresses are tainted since they come from a user */
-      uschar * s = string_copy_taint(argv[recipients_arg++], TRUE);
+      uschar * s = string_copy_taint(exim_str_fail_toolong(argv[recipients_arg++], EXIM_DISPLAYMAIL_MAX, "address verification"), TRUE);
       while (*s)
         {
         BOOL finished = FALSE;
@@ -5008,7 +5077,7 @@ if (verify_address_mode || f.address_test_mode)
     {
     uschar * s = get_stdinput(NULL, NULL);
     if (!s) break;
-    test_address(string_copy_taint(s, TRUE), flags, &exit_value);
+    test_address(string_copy_taint(exim_str_fail_toolong(s, EXIM_DISPLAYMAIL_MAX, "address verification (stdin)"), TRUE), flags, &exit_value);
     }
 
   route_tidyup();
@@ -5025,11 +5094,15 @@ if (expansion_test)
   dns_init(FALSE, FALSE, FALSE);
   if (msg_action_arg > 0 && msg_action == MSG_LOAD)
     {
-    uschar spoolname[256];  /* Not big_buffer; used in spool_read_header() */
+    uschar * spoolname;
     if (!f.admin_user)
       exim_fail("exim: permission denied\n");
-    message_id = argv[msg_action_arg];
-    (void)string_format(spoolname, sizeof(spoolname), "%s-H", message_id);
+    message_id = US exim_str_fail_toolong(argv[msg_action_arg], MESSAGE_ID_LENGTH, "message-id");
+    /* Checking the length of the ID is sufficient to validate it.
+    Get an untainted version so file opens can be done. */
+    message_id = string_copy_taint(message_id, FALSE);
+
+    spoolname = string_sprintf("%s-H", message_id);
     if ((deliver_datafile = spool_open_datafile(message_id)) < 0)
       printf ("Failed to load message datafile %s\n", message_id);
     if (spool_read_header(spoolname, TRUE, FALSE) != spool_read_OK)
@@ -5068,7 +5141,7 @@ if (expansion_test)
 
   if (recipients_arg < argc)
     while (recipients_arg < argc)
-      expansion_test_line(argv[recipients_arg++]);
+      expansion_test_line(exim_str_fail_toolong(argv[recipients_arg++], EXIM_EMAILADDR_MAX, "recipient"));
 
   /* Read stdin */
 
@@ -5406,15 +5479,15 @@ that SIG_IGN works. */
 
 if (!f.synchronous_delivery)
   {
-  #ifdef SA_NOCLDWAIT
+#ifdef SA_NOCLDWAIT
   struct sigaction act;
   act.sa_handler = SIG_IGN;
   sigemptyset(&(act.sa_mask));
   act.sa_flags = SA_NOCLDWAIT;
   sigaction(SIGCHLD, &act, NULL);
-  #else
+#else
   signal(SIGCHLD, SIG_IGN);
-  #endif
+#endif
   }
 
 /* Save the current store pool point, for resetting at the start of
@@ -5426,7 +5499,7 @@ real_sender_address = sender_address;
 messages to be read (SMTP input), or FALSE otherwise (not SMTP, or SMTP channel
 collapsed). */
 
-while (more)
+for (BOOL more = TRUE; more; )
   {
   rmark reset_point = store_mark();
   message_id[0] = 0;
@@ -5468,10 +5541,10 @@ while (more)
       /* Now get the data for the message */
 
       more = receive_msg(extract_recipients);
-      if (message_id[0] == 0)
+      if (!message_id[0])
         {
        cancel_cutthrough_connection(TRUE, US"receive dropped");
-        if (more) goto moreloop;
+        if (more) goto MORELOOP;
         smtp_log_no_mail();               /* Log no mail if configured */
         exim_exit(EXIT_FAILURE);
         }
@@ -5511,7 +5584,9 @@ while (more)
       {
       int start, end, domain;
       uschar * errmess;
-      uschar * s = string_copy_taint(list[i], TRUE);
+      /* There can be multiple addresses, so EXIM_DISPLAYMAIL_MAX (tuned for 1) is too short.
+       * We'll still want to cap it to something, just in case. */
+      uschar * s = string_copy_taint(exim_str_fail_toolong(list[i], BIG_BUFFER_SIZE, "address argument"), TRUE);
 
       /* Loop for each comma-separated address */
 
@@ -5546,11 +5621,12 @@ while (more)
           parse_extract_address(s, &errmess, &start, &end, &domain, FALSE);
 
 #ifdef SUPPORT_I18N
-       if (string_is_utf8(recipient))
-         message_smtputf8 = TRUE;
-       else
-         allow_utf8_domains = b;
+        if (recipient)
+          if (string_is_utf8(recipient)) message_smtputf8 = TRUE;
+          else allow_utf8_domains = b;
        }
+#else
+        ;
 #endif
         if (domain == 0 && !f.allow_unqualified_recipient)
           {
@@ -5634,7 +5710,7 @@ while (more)
     for real; when reading the headers of a message for filter testing,
     it is TRUE if the headers were terminated by '.' and FALSE otherwise. */
 
-    if (message_id[0] == 0) exim_exit(EXIT_FAILURE);
+    if (!message_id[0]) exim_exit(EXIT_FAILURE);
     }  /* Non-SMTP message reception */
 
   /* If this is a filter testing run, there are headers in store, but
@@ -5648,10 +5724,10 @@ while (more)
     {
     deliver_domain = ftest_domain ? ftest_domain : qualify_domain_recipient;
     deliver_domain_orig = deliver_domain;
-    deliver_localpart = ftest_localpart ? ftest_localpart : originator_login;
+    deliver_localpart = ftest_localpart ? US ftest_localpart : originator_login;
     deliver_localpart_orig = deliver_localpart;
-    deliver_localpart_prefix = ftest_prefix;
-    deliver_localpart_suffix = ftest_suffix;
+    deliver_localpart_prefix = US ftest_prefix;
+    deliver_localpart_suffix = US ftest_suffix;
     deliver_home = originator_home;
 
     if (!return_path)
@@ -5827,11 +5903,11 @@ while (more)
   finished subprocesses here, in case there are lots of messages coming in
   from the same source. */
 
-  #ifndef SIG_IGN_WORKS
+#ifndef SIG_IGN_WORKS
   while (waitpid(-1, NULL, WNOHANG) > 0);
-  #endif
+#endif
 
-moreloop:
+MORELOOP:
   return_path = sender_address = NULL;
   authenticated_sender = NULL;
   deliver_localpart_orig = NULL;