CVE-2020-28007: Link attack in Exim's log directory
[exim.git] / src / src / log.c
index 2832f4ed009b6056f7595f752cb335b5893f2b3a..5d36b498391aececef408921927ced34286d0010 100644 (file)
@@ -3,6 +3,7 @@
 *************************************************/
 
 /* Copyright (c) University of Cambridge 1995 - 2018 */
+/* Copyright (c) The Exim Maintainers 2020 */
 /* See the file NOTICE for conditions of use and distribution. */
 
 /* Functions for writing log files. The code for maintaining datestamped
@@ -112,6 +113,7 @@ static const uschar * exim_errstrings[] = {
   US"Local-only delivery",
   US"Domain in queue_domains",
   US"Transport concurrency limit",
+  US"Event requests alternate response",
 };
 
 
@@ -142,7 +144,7 @@ Returns:         nothing
 static void
 write_syslog(int priority, const uschar *s)
 {
-int len, pass;
+int len;
 int linecount = 0;
 
 if (!syslog_pid && LOGGING(pid))
@@ -171,12 +173,10 @@ if (!syslog_open && !f.running_in_test_harness)
 /* First do a scan through the message in order to determine how many lines
 it is going to end up as. Then rescan to output it. */
 
-for (pass = 0; pass < 2; pass++)
+for (int pass = 0; pass < 2; pass++)
   {
-  int i;
-  int tlen;
   const uschar * ss = s;
-  for (i = 1, tlen = len; tlen > 0; i++)
+  for (int i = 1, tlen = len; tlen > 0; i++)
     {
     int plen = tlen;
     uschar *nlptr = Ustrchr(ss, '\n');
@@ -243,7 +243,7 @@ if (s1)
   }
 if (f.receive_call_bombout) receive_bomb_out(NULL, s2);  /* does not return */
 if (smtp_input) smtp_closedown(s2);
-exim_exit(EXIT_FAILURE, NULL);
+exim_exit(EXIT_FAILURE);
 }
 
 
@@ -264,14 +264,19 @@ overwrite it temporarily if it is necessary to create the directory.
 Returns:       a file descriptor, or < 0 on failure (errno set)
 */
 
-int
-log_create(uschar *name)
+static int
+log_open_already_exim(uschar * const name)
 {
-int fd = Uopen(name,
-#ifdef O_CLOEXEC
-       O_CLOEXEC |
-#endif
-       O_CREAT|O_APPEND|O_WRONLY, LOG_MODE);
+int fd = -1;
+const int flags = O_WRONLY | O_APPEND | O_CREAT | O_NONBLOCK;
+
+if (geteuid() != exim_uid)
+  {
+  errno = EACCES;
+  return -1;
+  }
+
+fd = Uopen(name, flags, LOG_MODE);
 
 /* If creation failed, attempt to build a log directory in case that is the
 problem. */
@@ -285,11 +290,7 @@ if (fd < 0 && errno == ENOENT)
   DEBUG(D_any) debug_printf("%s log directory %s\n",
     created ? "created" : "failed to create", name);
   *lastslash = '/';
-  if (created) fd = Uopen(name,
-#ifdef O_CLOEXEC
-                       O_CLOEXEC |
-#endif
-                       O_CREAT|O_APPEND|O_WRONLY, LOG_MODE);
+  if (created) fd = Uopen(name, flags, LOG_MODE);
   }
 
 return fd;
@@ -297,6 +298,81 @@ return fd;
 
 
 
+/* Inspired by OpenSSH's mm_send_fd(). Thanks! */
+
+static int
+log_send_fd(const int sock, const int fd)
+{
+struct msghdr msg;
+union {
+  struct cmsghdr hdr;
+  char buf[CMSG_SPACE(sizeof(int))];
+} cmsgbuf;
+struct cmsghdr *cmsg;
+struct iovec vec;
+char ch = 'A';
+ssize_t n;
+
+memset(&msg, 0, sizeof(msg));
+memset(&cmsgbuf, 0, sizeof(cmsgbuf));
+msg.msg_control = &cmsgbuf.buf;
+msg.msg_controllen = sizeof(cmsgbuf.buf);
+
+cmsg = CMSG_FIRSTHDR(&msg);
+cmsg->cmsg_len = CMSG_LEN(sizeof(int));
+cmsg->cmsg_level = SOL_SOCKET;
+cmsg->cmsg_type = SCM_RIGHTS;
+*(int *)CMSG_DATA(cmsg) = fd;
+
+vec.iov_base = &ch;
+vec.iov_len = 1;
+msg.msg_iov = &vec;
+msg.msg_iovlen = 1;
+
+while ((n = sendmsg(sock, &msg, 0)) == -1 && errno == EINTR);
+if (n != 1) return -1;
+return 0;
+}
+
+/* Inspired by OpenSSH's mm_receive_fd(). Thanks! */
+
+static int
+log_recv_fd(const int sock)
+{
+struct msghdr msg;
+union {
+  struct cmsghdr hdr;
+  char buf[CMSG_SPACE(sizeof(int))];
+} cmsgbuf;
+struct cmsghdr *cmsg;
+struct iovec vec;
+ssize_t n;
+char ch = '\0';
+int fd = -1;
+
+memset(&msg, 0, sizeof(msg));
+vec.iov_base = &ch;
+vec.iov_len = 1;
+msg.msg_iov = &vec;
+msg.msg_iovlen = 1;
+
+memset(&cmsgbuf, 0, sizeof(cmsgbuf));
+msg.msg_control = &cmsgbuf.buf;
+msg.msg_controllen = sizeof(cmsgbuf.buf);
+
+while ((n = recvmsg(sock, &msg, 0)) == -1 && errno == EINTR);
+if (n != 1 || ch != 'A') return -1;
+
+cmsg = CMSG_FIRSTHDR(&msg);
+if (cmsg == NULL) return -1;
+if (cmsg->cmsg_type != SCM_RIGHTS) return -1;
+fd = *(const int *)CMSG_DATA(cmsg);
+if (fd < 0) return -1;
+return fd;
+}
+
+
+
 /*************************************************
 *     Create a log file as the exim user         *
 *************************************************/
@@ -312,41 +388,60 @@ Returns:       a file descriptor, or < 0 on failure (errno set)
 */
 
 int
-log_create_as_exim(uschar *name)
+log_open_as_exim(uschar * const name)
 {
-pid_t pid = fork();
-int status = 1;
 int fd = -1;
+const uid_t euid = geteuid();
 
-/* In the subprocess, change uid/gid and do the creation. Return 0 from the
-subprocess on success. If we don't check for setuid failures, then the file
-can be created as root, so vulnerabilities which cause setuid to fail mean
-that the Exim user can use symlinks to cause a file to be opened/created as
-root. We always open for append, so can't nuke existing content but it would
-still be Rather Bad. */
-
-if (pid == 0)
+if (euid == exim_uid)
   {
-  if (setgid(exim_gid) < 0)
-    die(US"exim: setgid for log-file creation failed, aborting",
-      US"Unexpected log failure, please try later");
-  if (setuid(exim_uid) < 0)
-    die(US"exim: setuid for log-file creation failed, aborting",
-      US"Unexpected log failure, please try later");
-  _exit((log_create(name) < 0)? 1 : 0);
+  fd = log_open_already_exim(name);
   }
+else if (euid == root_uid)
+  {
+  int sock[2];
+  if (socketpair(AF_UNIX, SOCK_STREAM, 0, sock) == 0)
+    {
+    const pid_t pid = exim_fork(US"logfile-open");
+    if (pid == 0)
+      {
+      (void)close(sock[0]);
+      if (setgroups(1, &exim_gid) != 0) _exit(EXIT_FAILURE);
+      if (setgid(exim_gid) != 0) _exit(EXIT_FAILURE);
+      if (setuid(exim_uid) != 0) _exit(EXIT_FAILURE);
+
+      if (getuid() != exim_uid || geteuid() != exim_uid) _exit(EXIT_FAILURE);
+      if (getgid() != exim_gid || getegid() != exim_gid) _exit(EXIT_FAILURE);
+
+      fd = log_open_already_exim(name);
+      if (fd < 0) _exit(EXIT_FAILURE);
+      if (log_send_fd(sock[1], fd) != 0) _exit(EXIT_FAILURE);
+      (void)close(sock[1]);
+      _exit(EXIT_SUCCESS);
+      }
 
-/* If we created a subprocess, wait for it. If it succeeded, try the open. */
-
-while (pid > 0 && waitpid(pid, &status, 0) != pid);
-if (status == 0) fd = Uopen(name,
-#ifdef O_CLOEXEC
-                       O_CLOEXEC |
-#endif
-                       O_APPEND|O_WRONLY, LOG_MODE);
+    (void)close(sock[1]);
+    if (pid > 0)
+      {
+      fd = log_recv_fd(sock[0]);
+      while (waitpid(pid, NULL, 0) == -1 && errno == EINTR);
+      }
+    (void)close(sock[0]);
+    }
+  }
 
-/* If we failed to create a subprocess, we are in a bad way. We return
-with fd still < 0, and errno set, letting the caller handle the error. */
+if (fd >= 0)
+  {
+  int flags;
+  flags = fcntl(fd, F_GETFD);
+  if (flags != -1) (void)fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
+  flags = fcntl(fd, F_GETFL);
+  if (flags != -1) (void)fcntl(fd, F_SETFL, flags & ~O_NONBLOCK);
+  }
+else
+  {
+  errno = EACCES;
+  }
 
 return fd;
 }
@@ -459,59 +554,24 @@ if (!ok)
   die(US"exim: log file path too long: aborting",
       US"Logging failure; please try later");
 
-/* We now have the file name. Try to open an existing file. After a successful
-open, arrange for automatic closure on exec(), and then return. */
+/* We now have the file name. After a successful open, return. */
 
-*fd = Uopen(buffer,
-#ifdef O_CLOEXEC
-               O_CLOEXEC |
-#endif
-               O_APPEND|O_WRONLY, LOG_MODE);
+*fd = log_open_as_exim(buffer);
 
 if (*fd >= 0)
   {
-#ifndef O_CLOEXEC
-  (void)fcntl(*fd, F_SETFD, fcntl(*fd, F_GETFD) | FD_CLOEXEC);
-#endif
   return;
   }
 
-/* Open was not successful: try creating the file. If this is a root process,
-we must do the creating in a subprocess set to exim:exim in order to ensure
-that the file is created with the right ownership. Otherwise, there can be a
-race if another Exim process is trying to write to the log at the same time.
-The use of SIGUSR1 by the exiwhat utility can provoke a lot of simultaneous
-writing. */
-
 euid = geteuid();
 
-/* If we are already running as the Exim user (even if that user is root),
-we can go ahead and create in the current process. */
-
-if (euid == exim_uid) *fd = log_create(buffer);
-
-/* Otherwise, if we are root, do the creation in an exim:exim subprocess. If we
-are neither exim nor root, creation is not attempted. */
-
-else if (euid == root_uid) *fd = log_create_as_exim(buffer);
-
-/* If we now have an open file, set the close-on-exec flag and return. */
-
-if (*fd >= 0)
-  {
-#ifndef O_CLOEXEC
-  (void)fcntl(*fd, F_SETFD, fcntl(*fd, F_GETFD) | FD_CLOEXEC);
-#endif
-  return;
-  }
-
 /* Creation failed. There are some circumstances in which we get here when
 the effective uid is not root or exim, which is the problem. (For example, a
 non-setuid binary with log_arguments set, called in certain ways.) Rather than
 just bombing out, force the log to stderr and carry on if stderr is available.
 */
 
-if (euid != root_uid && euid != exim_uid && log_stderr != NULL)
+if (euid != root_uid && euid != exim_uid && log_stderr)
   {
   *fd = fileno(log_stderr);
   return;
@@ -520,7 +580,9 @@ if (euid != root_uid && euid != exim_uid && log_stderr != NULL)
 /* Otherwise this is a disaster. This call is deliberately ONLY to the panic
 log. If possible, save a copy of the original line that was being logged. If we
 are recursing (can't open the panic log either), the pointer will already be
-set. */
+set.  Also, when we had to use a subprocess for the create we didn't retrieve
+errno from it, so get the error from the open attempt above (which is often
+meaningful enough, so leave it). */
 
 if (!panic_save_buffer)
   if ((panic_save_buffer = US malloc(LOG_BUFFER_SIZE)))
@@ -554,23 +616,18 @@ Arguments:
 Returns:      updated pointer
 */
 
-static uschar *
-log_config_info(uschar *ptr, int flags)
+static gstring *
+log_config_info(gstring * g, int flags)
 {
-Ustrcpy(ptr, "Exim configuration error");
-ptr += 24;
+g = string_cat(g, US"Exim configuration error");
 
-if ((flags & (LOG_CONFIG_FOR & ~LOG_CONFIG)) != 0)
-  {
-  Ustrcpy(ptr, " for ");
-  return ptr + 5;
-  }
+if (flags & (LOG_CONFIG_FOR & ~LOG_CONFIG))
+  return string_cat(g, US" for ");
 
-if ((flags & (LOG_CONFIG_IN & ~LOG_CONFIG)) != 0)
-  ptr += sprintf(CS ptr, " in line %d of %s", config_lineno, config_filename);
+if (flags & (LOG_CONFIG_IN & ~LOG_CONFIG))
+  g = string_fmt_append(g, " in line %d of %s", config_lineno, config_filename);
 
-Ustrcpy(ptr, ":\n  ");
-return ptr + 4;
+return string_catn(g, US":\n  ", 4);
 }
 
 
@@ -741,10 +798,10 @@ Returns:    nothing
 void
 log_write(unsigned int selector, int flags, const char *format, ...)
 {
-uschar * ptr;
-int length;
 int paniclogfd;
 ssize_t written_len;
+gstring gs = { .size = LOG_BUFFER_SIZE-1, .ptr = 0, .s = log_buffer };
+gstring * g;
 va_list ap;
 
 /* If panic_recurseflag is set, we have failed to open the panic log. This is
@@ -771,7 +828,7 @@ if (!log_buffer)
   if (!(log_buffer = US malloc(LOG_BUFFER_SIZE)))
     {
     fprintf(stderr, "exim: failed to get store for log buffer\n");
-    exim_exit(EXIT_FAILURE, NULL);
+    exim_exit(EXIT_FAILURE);
     }
 
 /* If we haven't already done so, inspect the setting of log_file_path to
@@ -851,10 +908,8 @@ in one go so that it doesn't get split when multi-processing. */
 DEBUG(D_any|D_v)
   {
   int i;
-  ptr = log_buffer;
 
-  Ustrcpy(ptr, "LOG:");
-  ptr += 4;
+  g = string_catn(&gs, US"LOG:", 4);
 
   /* Show the selector that was passed into the call. */
 
@@ -862,31 +917,38 @@ DEBUG(D_any|D_v)
     {
     unsigned int bitnum = log_options[i].bit;
     if (bitnum < BITWORDSIZE && selector == BIT(bitnum))
-      {
-      *ptr++ = ' ';
-      Ustrcpy(ptr, log_options[i].name);
-      while (*ptr) ptr++;
-      }
+      g = string_fmt_append(g, " %s", log_options[i].name);
     }
 
-  ptr += sprintf(CS ptr, "%s%s%s%s\n  ",
+  g = string_fmt_append(g, "%s%s%s%s\n  ",
     flags & LOG_MAIN ?    " MAIN"   : "",
     flags & LOG_PANIC ?   " PANIC"  : "",
     (flags & LOG_PANIC_DIE) == LOG_PANIC_DIE ? " DIE" : "",
     flags & LOG_REJECT ?  " REJECT" : "");
 
-  if (flags & LOG_CONFIG) ptr = log_config_info(ptr, flags);
+  if (flags & LOG_CONFIG) g = log_config_info(g, flags);
+
+  /* We want to be able to log tainted info, but log_buffer is directly
+  malloc'd.  So use deliberately taint-nonchecking routines to build into
+  it, trusting that we will never expand the results. */
 
   va_start(ap, format);
-  if (!string_vformat(ptr, LOG_BUFFER_SIZE - (ptr-log_buffer)-1, format, ap))
-    Ustrcpy(ptr, "**** log string overflowed log buffer ****");
+  i = g->ptr;
+  if (!string_vformat(g, SVFMT_TAINT_NOCHK, format, ap))
+    {
+    g->ptr = i;
+    g = string_cat(g, US"**** log string overflowed log buffer ****");
+    }
   va_end(ap);
 
-  while(*ptr) ptr++;
-  Ustrcat(ptr, "\n");
-  debug_printf("%s", log_buffer);
-  }
+  g->size = LOG_BUFFER_SIZE;
+  g = string_catn(g, US"\n", 1);
+  debug_printf("%s", string_from_gstring(g));
 
+  gs.size = LOG_BUFFER_SIZE-1; /* Having used the buffer for debug output, */
+  gs.ptr = 0;                  /* reset it for the real use. */
+  gs.s = log_buffer;
+  }
 /* If no log file is specified, we are in a mess. */
 
 if (!(flags & (LOG_MAIN|LOG_PANIC|LOG_REJECT)))
@@ -898,6 +960,7 @@ if (!(flags & (LOG_MAIN|LOG_PANIC|LOG_REJECT)))
 if (f.disable_logging)
   {
   DEBUG(D_any) debug_printf("log writing disabled\n");
+  if ((flags & LOG_PANIC_DIE) == LOG_PANIC_DIE) exim_exit(EXIT_FAILURE);
   return;
   }
 
@@ -908,54 +971,64 @@ if (!write_rejectlog) flags &= ~LOG_REJECT;
 /* Create the main message in the log buffer. Do not include the message id
 when called by a utility. */
 
-ptr = log_buffer;
-ptr += sprintf(CS ptr, "%s ", tod_stamp(tod_log));
+g = string_fmt_append(&gs, "%s ", tod_stamp(tod_log));
 
 if (LOGGING(pid))
   {
-  if (!syslog_pid) pid_position[0] = ptr - log_buffer; /* remember begin … */
-  ptr += sprintf(CS ptr, "[%d] ", (int)getpid());
-  if (!syslog_pid) pid_position[1] = ptr - log_buffer; /*  … and end+1 of the PID */
+  if (!syslog_pid) pid_position[0] = g->ptr;           /* remember begin … */
+  g = string_fmt_append(g, "[%d] ", (int)getpid());
+  if (!syslog_pid) pid_position[1] = g->ptr;           /*  … and end+1 of the PID */
   }
 
 if (f.really_exim && message_id[0] != 0)
-  ptr += sprintf(CS ptr, "%s ", message_id);
+  g = string_fmt_append(g, "%s ", message_id);
 
-if (flags & LOG_CONFIG) ptr = log_config_info(ptr, flags);
+if (flags & LOG_CONFIG)
+  g = log_config_info(g, flags);
 
 va_start(ap, format);
-if (!string_vformat(ptr, LOG_BUFFER_SIZE - (ptr-log_buffer)-1, format, ap))
-  Ustrcpy(ptr, "**** log string overflowed log buffer ****\n");
-while(*ptr) ptr++;
+  {
+  int i = g->ptr;
+
+  /* We want to be able to log tainted info, but log_buffer is directly
+  malloc'd.  So use deliberately taint-nonchecking routines to build into
+  it, trusting that we will never expand the results. */
+
+  if (!string_vformat(g, SVFMT_TAINT_NOCHK, format, ap))
+    {
+    g->ptr = i;
+    g = string_cat(g, US"**** log string overflowed log buffer ****\n");
+    }
+  }
 va_end(ap);
 
 /* Add the raw, unrewritten, sender to the message if required. This is done
 this way because it kind of fits with LOG_RECIPIENTS. */
 
 if (   flags & LOG_SENDER
-    && ptr < log_buffer + LOG_BUFFER_SIZE - 10 - Ustrlen(raw_sender))
-  ptr += sprintf(CS ptr, " from <%s>", raw_sender);
+   && g->ptr < LOG_BUFFER_SIZE - 10 - Ustrlen(raw_sender))
+  g = string_fmt_append_f(g, SVFMT_TAINT_NOCHK, " from <%s>", raw_sender);
 
 /* Add list of recipients to the message if required; the raw list,
 before rewriting, was saved in raw_recipients. There may be none, if an ACL
 discarded them all. */
 
 if (  flags & LOG_RECIPIENTS
-   && ptr < log_buffer + LOG_BUFFER_SIZE - 6
+   && g->ptr < LOG_BUFFER_SIZE - 6
    && raw_recipients_count > 0)
   {
   int i;
-  ptr += sprintf(CS ptr, " for");
+  g = string_fmt_append_f(g, SVFMT_TAINT_NOCHK, " for", NULL);
   for (i = 0; i < raw_recipients_count; i++)
     {
     uschar * s = raw_recipients[i];
-    if (log_buffer + LOG_BUFFER_SIZE - ptr < Ustrlen(s) + 3) break;
-    ptr += sprintf(CS ptr, " %s", s);
+    if (LOG_BUFFER_SIZE - g->ptr < Ustrlen(s) + 3) break;
+    g = string_fmt_append_f(g, SVFMT_TAINT_NOCHK, " %s", s);
     }
   }
 
-ptr += sprintf(CS  ptr, "\n");
-length = ptr - log_buffer;
+g = string_catn(g, US"\n", 1);
+string_from_gstring(g);
 
 /* Handle loggable errors when running a utility, or when address testing.
 Write to log_stderr unless debugging (when it will already have been written),
@@ -972,7 +1045,7 @@ if (!f.really_exim || f.log_testing_mode)
     else
       fprintf(log_stderr, "%s", CS log_buffer);
 
-  if ((flags & LOG_PANIC_DIE) == LOG_PANIC_DIE) exim_exit(EXIT_FAILURE, US"");
+  if ((flags & LOG_PANIC_DIE) == LOG_PANIC_DIE) exim_exit(EXIT_FAILURE);
   return;
   }
 
@@ -1028,10 +1101,10 @@ if (  flags & LOG_MAIN
 
     /* Failing to write to the log is disastrous */
 
-    written_len = write_to_fd_buf(mainlogfd, log_buffer, length);
-    if (written_len != length)
+    written_len = write_to_fd_buf(mainlogfd, g->s, g->ptr);
+    if (written_len != g->ptr)
       {
-      log_write_failed(US"main log", length, written_len);
+      log_write_failed(US"main log", g->ptr, written_len);
       /* That function does not return */
       }
     }
@@ -1044,66 +1117,62 @@ headers. */
 
 if (flags & LOG_REJECT)
   {
-  header_line *h;
-
   if (header_list && LOGGING(rejected_header))
     {
+    gstring * g2;
+    int i;
+
     if (recipients_count > 0)
       {
-      int i;
-
       /* List the sender */
 
-      string_format(ptr, LOG_BUFFER_SIZE - (ptr-log_buffer),
-        "Envelope-from: <%s>\n", sender_address);
-      while (*ptr) ptr++;
+      g2 = string_fmt_append_f(g, SVFMT_TAINT_NOCHK,
+                       "Envelope-from: <%s>\n", sender_address);
+      if (g2) g = g2;
 
       /* List up to 5 recipients */
 
-      string_format(ptr, LOG_BUFFER_SIZE - (ptr-log_buffer),
-        "Envelope-to: <%s>\n", recipients_list[0].address);
-      while (*ptr) ptr++;
+      g2 = string_fmt_append_f(g, SVFMT_TAINT_NOCHK,
+                       "Envelope-to: <%s>\n", recipients_list[0].address);
+      if (g2) g = g2;
 
       for (i = 1; i < recipients_count && i < 5; i++)
         {
-        string_format(ptr, LOG_BUFFER_SIZE - (ptr-log_buffer), "    <%s>\n",
-          recipients_list[i].address);
-        while (*ptr) ptr++;
+        g2 = string_fmt_append_f(g, SVFMT_TAINT_NOCHK,
+                       "    <%s>\n", recipients_list[i].address);
+       if (g2) g = g2;
         }
 
       if (i < recipients_count)
         {
-        (void)string_format(ptr, LOG_BUFFER_SIZE - (ptr-log_buffer),
-          "    ...\n");
-        while (*ptr) ptr++;
+        g2 = string_fmt_append_f(g, SVFMT_TAINT_NOCHK, "    ...\n", NULL);
+       if (g2) g = g2;
         }
       }
 
     /* A header with a NULL text is an unfilled in Received: header */
 
-    for (h = header_list; h; h = h->next) if (h->text)
+    for (header_line * h = header_list; h; h = h->next) if (h->text)
       {
-      BOOL fitted = string_format(ptr, LOG_BUFFER_SIZE - (ptr-log_buffer),
-        "%c %s", h->type, h->text);
-      while(*ptr) ptr++;
-      if (!fitted)         /* Buffer is full; truncate */
+      g2 = string_fmt_append_f(g, SVFMT_TAINT_NOCHK,
+                       "%c %s", h->type, h->text);
+      if (g2)
+       g = g2;
+      else             /* Buffer is full; truncate */
         {
-        ptr -= 100;        /* For message and separator */
-        if (ptr[-1] == '\n') ptr--;
-        Ustrcpy(ptr, "\n*** truncated ***\n");
-        while (*ptr) ptr++;
+        g->ptr -= 100;        /* For message and separator */
+        if (g->s[g->ptr-1] == '\n') g->ptr--;
+        g = string_cat(g, US"\n*** truncated ***\n");
         break;
         }
       }
-
-    length = ptr - log_buffer;
     }
 
   /* Write to syslog or to a log file */
 
   if (  logging_mode & LOG_MODE_SYSLOG
      && (syslog_duplication || !(flags & LOG_PANIC)))
-    write_syslog(LOG_NOTICE, log_buffer);
+    write_syslog(LOG_NOTICE, string_from_gstring(g));
 
   /* Check for a change to the rejectlog file name when datestamping is in
   operation. This happens at midnight, at which point we want to roll over
@@ -1147,10 +1216,10 @@ if (flags & LOG_REJECT)
       if (fstat(rejectlogfd, &statbuf) >= 0) rejectlog_inode = statbuf.st_ino;
       }
 
-    written_len = write_to_fd_buf(rejectlogfd, log_buffer, length);
-    if (written_len != length)
+    written_len = write_to_fd_buf(rejectlogfd, g->s, g->ptr);
+    if (written_len != g->ptr)
       {
-      log_write_failed(US"reject log", length, written_len);
+      log_write_failed(US"reject log", g->ptr, written_len);
       /* That function does not return */
       }
     }
@@ -1165,7 +1234,7 @@ all cases except mua_wrapper, try to write to log_stderr. */
 if (flags & LOG_PANIC)
   {
   if (log_stderr && log_stderr != debug_file && !mua_wrapper)
-    fprintf(log_stderr, "%s", CS log_buffer);
+    fprintf(log_stderr, "%s", CS string_from_gstring(g));
 
   if (logging_mode & LOG_MODE_SYSLOG)
     write_syslog(LOG_ALERT, log_buffer);
@@ -1185,14 +1254,14 @@ if (flags & LOG_PANIC)
       i = i;   /* compiler quietening */
       }
 
-    written_len = write_to_fd_buf(paniclogfd, log_buffer, length);
-    if (written_len != length)
+    written_len = write_to_fd_buf(paniclogfd, g->s, g->ptr);
+    if (written_len != g->ptr)
       {
       int save_errno = errno;
       write_syslog(LOG_CRIT, log_buffer);
       sprintf(CS log_buffer, "write failed on panic log: length=%d result=%d "
-        "errno=%d (%s)", length, (int)written_len, save_errno, strerror(save_errno));
-      write_syslog(LOG_CRIT, log_buffer);
+        "errno=%d (%s)", g->ptr, (int)written_len, save_errno, strerror(save_errno));
+      write_syslog(LOG_CRIT, string_from_gstring(g));
       flags |= LOG_PANIC_DIE;
       }
 
@@ -1291,14 +1360,14 @@ decode_bits(unsigned int *selector, size_t selsize, int *notall,
   uschar *string, bit_table *options, int count, uschar *which, int flags)
 {
 uschar *errmsg;
-if (string == NULL) return;
+if (!string) return;
 
 if (*string == '=')
   {
   char *end;    /* Not uschar */
   memset(selector, 0, sizeof(*selector)*selsize);
   *selector = strtoul(CS string+1, &end, 0);
-  if (*end == 0) return;
+  if (!*end) return;
   errmsg = string_sprintf("malformed numeric %s_selector setting: %s", which,
     string);
   goto ERROR_RETURN;
@@ -1313,8 +1382,8 @@ else for(;;)
   int len;
   bit_table *start, *end;
 
-  while (isspace(*string)) string++;
-  if (*string == 0) return;
+  Uskip_whitespace(&string);
+  if (!*string) return;
 
   if (*string != '+' && *string != '-')
     {
@@ -1336,7 +1405,6 @@ else for(;;)
     bit_table *middle = start + (end - start)/2;
     int c = Ustrncmp(s, middle->name, len);
     if (c == 0)
-      {
       if (middle->name[len] != 0) c = -1; else
         {
         unsigned int bit = middle->bit;
@@ -1358,7 +1426,6 @@ else for(;;)
 
         break;  /* Out of loop to match selector name */
         }
-      }
     if (c < 0) end = middle; else start = middle + 1;
     }  /* Loop to match selector name */