Tidying: coverity issues

6 files changed, 100 insertions, 55 deletions

diff --git a/src/OS/os.h-Linux b/src/OS/os.h-Linux
index 05c153e2c..6400e798e 100644
--- a/src/OS/os.h-Linux
+++ b/src/OS/os.h-Linux
@@ -65,5 +65,9 @@ then change the 0 to 1 in the next block. */
 # define LLONG_MAX LONG_LONG_MAX
 #endif
 
+#if _POSIX_C_SOURCE >= 200809L || _ATFILE_SOUCE
+# define EXIM_HAVE_OPENAT
+#endif
+
 
 /* End */
diff --git a/src/src/deliver.c b/src/src/deliver.c
index 6fee7be79..dc616a1db 100644
--- a/src/src/deliver.c
+++ b/src/src/deliver.c
@@ -1601,7 +1601,7 @@ else
   force the af_ignore_error flag. This will cause the address to be discarded
   later (with a log entry). */
 
-  if (sender_address[0] == 0 && message_age >= ignore_bounce_errors_after)
+  if (!*sender_address && message_age >= ignore_bounce_errors_after)
     setflag(addr, af_ignore_error);
 
   /* Freeze the message if requested, or if this is a bounce message (or other
@@ -5578,10 +5578,8 @@ if (deliver_freeze)
   ignore timer is exceeded. The message will be discarded if this delivery
   fails. */
 
-  else if (sender_address[0] == 0 && message_age >= ignore_bounce_errors_after)
-    {
+  else if (!*sender_address && message_age >= ignore_bounce_errors_after)
     log_write(0, LOG_MAIN, "Unfrozen by errmsg timer");
-    }
 
   /* If this is a bounce message, or there's no auto thaw, or we haven't
   reached the auto thaw time yet, and this delivery is not forced by an admin
diff --git a/src/src/parse.c b/src/src/parse.c
index d3f382b96..7a072558a 100644
--- a/src/src/parse.c
+++ b/src/src/parse.c
@@ -1425,7 +1425,7 @@ for (;;)
 
     /* Check file name if required */
 
-    if (directory != NULL)
+    if (directory)
       {
       int len = Ustrlen(directory);
       uschar *p = filename + len;
@@ -1437,16 +1437,52 @@ for (;;)
         return FF_ERROR;
         }
 
+#ifdef EXIM_HAVE_OPENAT
+      /* It is necessary to check that every component inside the directory
+      is NOT a symbolic link, in order to keep the file inside the directory.
+      This is mighty tedious. We open the directory and openat every component,
+      with a flag that fails symlinks. */
+
+      {
+      int fd = open(directory, O_RDONLY);
+      if (fd < 0)
+	{
+	*error = string_sprintf("failed to open directory %s", directory);
+	return FF_ERROR;
+	}
+      while (*p)
+	{
+	uschar temp;
+	int fd2;
+	uschar * q = p;
+
+	while (*++p && *p != '/') ;
+	temp = *p;
+	*p = '\0';
+
+	if ((fd2 = openat(fd, q, O_RDONLY|O_NOFOLLOW)) < 0)
+	  {
+          *error = string_sprintf("failed to open %s (component of included "
+            "file); could be symbolic link", filename);
+	  return FF_ERROR;
+	  }
+	close(fd);
+	fd = fd2;
+	*p = temp;
+	}
+      f = fdopen(fd, "rb");
+      }
+#else
       /* It is necessary to check that every component inside the directory
       is NOT a symbolic link, in order to keep the file inside the directory.
       This is mighty tedious. It is also not totally foolproof in that it
       leaves the possibility of a race attack, but I don't know how to do
       any better. */
 
-      while (*p != 0)
+      while (*p)
         {
         int temp;
-        while (*(++p) != 0 && *p != '/');
+        while (*++p && *p != '/');
         temp = *p;
         *p = 0;
         if (Ulstat(filename, &statbuf) != 0)
@@ -1466,11 +1502,16 @@ for (;;)
           return FF_ERROR;
           }
         }
+#endif
       }
 
-    /* Open and stat the file */
+#ifdef EXIM_HAVE_OPENAT
+    else
+#endif
+      /* Open and stat the file */
+      f = Ufopen(filename, "rb");
 
-    if ((f = Ufopen(filename, "rb")) == NULL)
+    if (!f)
       {
       *error = string_open_failed(errno, "included file %s", filename);
       return FF_INCLUDEFAIL;
@@ -1486,7 +1527,7 @@ for (;;)
 
     /* If directory was checked, double check that we opened a regular file */
 
-    if (directory != NULL && (statbuf.st_mode & S_IFMT) != S_IFREG)
+    if (directory && (statbuf.st_mode & S_IFMT) != S_IFREG)
       {
       *error = string_sprintf("included file %s is not a regular file in "
         "the %s directory", filename, directory);
@@ -1518,10 +1559,9 @@ for (;;)
       error, incoming_domain, directory, syntax_errors);
     if (frc != FF_DELIVERED && frc != FF_NOTDELIVERED) return frc;
 
-    if (addr != NULL)
+    if (addr)
       {
-      last = addr;
-      while (last->next != NULL) { count++; last = last->next; }
+      for (last = addr; last->next; last = last->next) count++;
       last->next = *anchor;
       *anchor = addr;
       count++;
diff --git a/src/src/pdkim/pdkim.c b/src/src/pdkim/pdkim.c
index bf2bbba6c..ccd7784f3 100644
--- a/src/src/pdkim/pdkim.c
+++ b/src/src/pdkim/pdkim.c
@@ -1014,7 +1014,7 @@ int p;
 if (!data)
   pdkim_body_complete(ctx);
 
-for (p = 0; p<len; p++)
+else for (p = 0; p<len; p++)
   {
   uschar c = data[p];
 
@@ -1309,8 +1309,6 @@ DLLEXPORT int
 pdkim_feed_finish(pdkim_ctx *ctx, pdkim_signature **return_signatures)
 {
 pdkim_signature *sig = ctx->sig;
-uschar * headernames = NULL;             /* Collected signed header names */
-int hs = 0, hl = 0;
 
 /* Check if we must still flush a (partial) header. If that is the
    case, the message has no body, and we must compute a body hash
@@ -1353,6 +1351,8 @@ while (sig)
 
   if (ctx->flags & PDKIM_MODE_SIGN)
     {
+    uschar * headernames = NULL;	/* Collected signed header names */
+    int hs = 0, hl = 0;
     pdkim_stringlist *p;
     const uschar * l;
     uschar * s;
@@ -1394,7 +1394,6 @@ while (sig)
 
     /* Copy headernames to signature struct */
     sig->headernames = headernames;
-    headernames = NULL, hs = hl = 0;
 
     /* Create signature header with b= omitted */
     sig_hdr = pdkim_create_header(sig, FALSE);
@@ -1405,44 +1404,49 @@ while (sig)
      add the headers to the hash in that order. */
   else
     {
-    uschar * b = string_copy(sig->headernames);
-    uschar * p = b;
+    uschar * p = sig->headernames;
     uschar * q;
     pdkim_stringlist * hdrs;
 
-    /* clear tags */
-    for (hdrs = ctx->headers; hdrs; hdrs = hdrs->next)
-      hdrs->tag = 0;
-
-    while(1)
+    if (p)
       {
-      if ((q = Ustrchr(p, ':')))
-	*q = '\0';
-
-/*XXX walk the list of headers in same order as received. */
+      /* clear tags */
       for (hdrs = ctx->headers; hdrs; hdrs = hdrs->next)
-	if (  hdrs->tag == 0
-	   && strncasecmp(hdrs->value, CS p, Ustrlen(p)) == 0
-	   && (hdrs->value)[Ustrlen(p)] == ':'
-	   )
-	  {
-	  uschar * rh = sig->canon_headers == PDKIM_CANON_RELAXED
-	    ? pdkim_relax_header(hdrs->value, 1) /* cook header for relaxed canon */
-	    : string_copy(CUS hdrs->value);      /* just copy it for simple canon */
+	hdrs->tag = 0;
 
-	  /* Feed header to the hash algorithm */
-	  exim_sha_update(&hhash_ctx, CUS rh, Ustrlen(rh));
+      p = string_copy(p);
+      while(1)
+	{
+	if ((q = Ustrchr(p, ':')))
+	  *q = '\0';
+
+  /*XXX walk the list of headers in same order as received. */
+	for (hdrs = ctx->headers; hdrs; hdrs = hdrs->next)
+	  if (  hdrs->tag == 0
+	     && strncasecmp(hdrs->value, CS p, Ustrlen(p)) == 0
+	     && (hdrs->value)[Ustrlen(p)] == ':'
+	     )
+	    {
+	    /* cook header for relaxed canon, or just copy it for simple  */
+
+	    uschar * rh = sig->canon_headers == PDKIM_CANON_RELAXED
+	      ? pdkim_relax_header(hdrs->value, 1)
+	      : string_copy(CUS hdrs->value);
+
+	    /* Feed header to the hash algorithm */
+	    exim_sha_update(&hhash_ctx, CUS rh, Ustrlen(rh));
+
+	    DEBUG(D_acl) pdkim_quoteprint(rh, Ustrlen(rh));
+	    hdrs->tag = 1;
+	    break;
+	    }
 
-	  DEBUG(D_acl) pdkim_quoteprint(rh, Ustrlen(rh));
-	  hdrs->tag = 1;
-	  break;
-	  }
+	if (!q) break;
+	p = q+1;
+	}
 
-      if (!q) break;
-      p = q+1;
+      sig_hdr = string_copy(sig->rawsig_no_b_val);
       }
-
-    sig_hdr = string_copy(sig->rawsig_no_b_val);
     }
 
   DEBUG(D_acl) debug_printf(
diff --git a/src/src/readconf.c b/src/src/readconf.c
index a1591e2a1..c145c7749 100644
--- a/src/src/readconf.c
+++ b/src/src/readconf.c
@@ -932,10 +932,10 @@ for (;;)
     save->filename = config_filename;
     save->lineno = config_lineno;
 
-    config_file = Ufopen(ss, "rb");
-    if (config_file == NULL)
+    if (!(config_file = Ufopen(ss, "rb")))
       log_write(0, LOG_PANIC_DIE|LOG_CONFIG_IN, "failed to open included "
         "configuration file %s", ss);
+
     config_filename = string_copy(ss);
     config_lineno = 0;
     continue;
diff --git a/src/src/spool_mbox.c b/src/src/spool_mbox.c
index b7ab06127..89691652e 100644
--- a/src/src/spool_mbox.c
+++ b/src/src/spool_mbox.c
@@ -165,15 +165,14 @@ if (!spool_mbox_ok)
   }
 
 /* get the size of the mbox message and open [message_id].eml file for reading*/
-if (Ustat(mbox_path, &statbuf) != 0 ||
-    (yield = Ufopen(mbox_path,"rb")) == NULL)
-  {
+
+if (  !(yield = Ufopen(mbox_path,"rb"))
+   || fstat(fileno(yield), &statbuf) != 0
+   )
   log_write(0, LOG_MAIN|LOG_PANIC, "%s", string_open_failed(errno,
     "scan file %s", mbox_path));
-  goto OUT;
-  }
-
-*mbox_file_size = statbuf.st_size;
+else
+  *mbox_file_size = statbuf.st_size;
 
 OUT:
 if (data_file) (void)fclose(data_file);