Tidying: coverity issues

7 files changed, 216 insertions, 150 deletions

diff --git a/src/src/auths/auth-spa.c b/src/src/auths/auth-spa.c
index ab01d53fa..d1df7f2cb 100644
--- a/src/src/auths/auth-spa.c
+++ b/src/src/auths/auth-spa.c
@@ -1262,6 +1262,9 @@ spa_bytes_add(ptr, header, b, len*2); \
 unicodeToString(((char*)structPtr) + IVAL(&structPtr->header.offset,0) , SVAL(&structPtr->header.len,0)/2)
 #define GetString(structPtr, header) \
 toString((((char *)structPtr) + IVAL(&structPtr->header.offset,0)), SVAL(&structPtr->header.len,0))
+
+#ifdef notdef
+
 #define DumpBuffer(fp, structPtr, header) \
 dumpRaw(fp,(US structPtr)+IVAL(&structPtr->header.offset,0),SVAL(&structPtr->header.len,0))
 
@@ -1277,6 +1280,8 @@ dumpRaw (FILE * fp, uschar *buf, size_t len)
   fprintf (fp, "\n");
 }
 
+#endif
+
 char *
 unicodeToString (char *p, size_t len)
 {
@@ -1325,6 +1330,8 @@ toString (char *p, size_t len)
   return buf;
 }
 
+#ifdef notdef
+
 void
 dumpSmbNtlmAuthRequest (FILE * fp, SPAAuthRequest * request)
 {
@@ -1365,6 +1372,7 @@ dumpSmbNtlmAuthResponse (FILE * fp, SPAAuthResponse * response)
   DumpBuffer (fp, response, sessionKey);
   fprintf (fp, "      Flags = %08x\n", IVAL (&response->flags, 0));
 }
+#endif
 
 void
 spa_build_auth_request (SPAAuthRequest * request, char *user, char *domain)
diff --git a/src/src/deliver.c b/src/src/deliver.c
index eb6c70515..f596cb684 100644
--- a/src/src/deliver.c
+++ b/src/src/deliver.c
@@ -2402,8 +2402,7 @@ will remain. Afterwards, close the reading end. */
 
 for (addr2 = addr; addr2; addr2 = addr2->next)
   {
-  len = read(pfd[pipe_read], &status, sizeof(int));
-  if (len > 0)
+  if ((len = read(pfd[pipe_read], &status, sizeof(int))) > 0)
     {
     int i;
     uschar **sptr;
@@ -2420,10 +2419,24 @@ for (addr2 = addr; addr2; addr2 = addr2->next)
 
     if (testflag(addr2, af_file))
       {
-      int local_part_length;
-      len = read(pfd[pipe_read], &local_part_length, sizeof(int));
-      len = read(pfd[pipe_read], big_buffer, local_part_length);
-      big_buffer[local_part_length] = 0;
+      int llen;
+      if (  read(pfd[pipe_read], &llen, sizeof(int)) != sizeof(int)
+	 || llen > 64*4	/* limit from rfc 5821, times I18N factor */
+         )
+	{
+	log_write(0, LOG_MAIN|LOG_PANIC, "bad local_part length read"
+	  " from delivery subprocess");
+	break;
+	}
+      /* sanity-checked llen so disable the Coverity error */
+      /* coverity[tainted_data] */
+      if (read(pfd[pipe_read], big_buffer, llen) != llen)
+	{
+	log_write(0, LOG_MAIN|LOG_PANIC, "bad local_part read"
+	  " from delivery subprocess");
+	break;
+	}
+      big_buffer[llen] = 0;
       addr2->local_part = string_copy(big_buffer);
       }
 
diff --git a/src/src/exim_dbmbuild.c b/src/src/exim_dbmbuild.c
index 7babc643e..611b6be38 100644
--- a/src/src/exim_dbmbuild.c
+++ b/src/src/exim_dbmbuild.c
@@ -478,9 +478,11 @@ if (yield == 0 || yield == 1)
 else
   {
   printf("dbmbuild abandoned\n");
-  #if defined(USE_DB) || defined(USE_TDB) || defined(USE_GDBM)
+#if defined(USE_DB) || defined(USE_TDB) || defined(USE_GDBM)
+  /* We created it, so safe to delete despite the name coming from outside */
+  /* coverity[tainted_data] */
   Uunlink(temp_dbmname);
-  #else
+#else
   if (is_db)
     {
     sprintf(CS real_dbmname, "%s.db", temp_dbmname);
@@ -493,7 +495,7 @@ else
     sprintf(CS real_dbmname, "%s.pag", temp_dbmname);
     Uunlink(real_dbmname);
     }
-  #endif /* USE_DB || USE_TDB */
+#endif /* USE_DB || USE_TDB */
   }
 
 return yield;
diff --git a/src/src/lookups/cdb.c b/src/src/lookups/cdb.c
index 4ff42ab3e..3a9078a4e 100644
--- a/src/src/lookups/cdb.c
+++ b/src/src/lookups/cdb.c
@@ -281,135 +281,171 @@ cdb_find(void *handle,
         uschar **errmsg,
         uint *do_cache)
 {
-  struct cdb_state * cdbp = handle;
-  uint32 item_key_len,
-    item_dat_len,
-    key_hash,
-    item_hash,
-    item_posn,
-    cur_offset,
-    end_offset,
-    hash_offset_entry,
-    hash_offset,
-    hash_offlen,
-    hash_slotnm;
-  int loop;
-
-  /* Keep picky compilers happy */
-  do_cache = do_cache;
-
-  key_hash = cdb_hash(keystring, key_len);
-
-  hash_offset_entry = CDB_HASH_ENTRY * (key_hash & CDB_HASH_MASK);
-  hash_offset = cdb_unpack(cdbp->cdb_offsets + hash_offset_entry);
-  hash_offlen = cdb_unpack(cdbp->cdb_offsets + hash_offset_entry + 4);
-
-  /* If the offset length is zero this key cannot be in the file */
-  if (hash_offlen == 0) {
-    return FAIL;
-  }
-  hash_slotnm = (key_hash >> 8) % hash_offlen;
-
-  /* check to ensure that the file is not corrupt
-   * if the hash_offset + (hash_offlen * CDB_HASH_ENTRY) is longer
-   * than the file, then we have problems.... */
-  if ((hash_offset + (hash_offlen * CDB_HASH_ENTRY)) > cdbp->filelen) {
-    *errmsg = string_sprintf("cdb: corrupt cdb file %s (too short)",
-                            filename);
-    DEBUG(D_lookup) debug_printf("%s\n", *errmsg);
-    return DEFER;
+struct cdb_state * cdbp = handle;
+uint32 item_key_len,
+item_dat_len,
+key_hash,
+item_hash,
+item_posn,
+cur_offset,
+end_offset,
+hash_offset_entry,
+hash_offset,
+hash_offlen,
+hash_slotnm;
+int loop;
+
+/* Keep picky compilers happy */
+do_cache = do_cache;
+
+key_hash = cdb_hash(keystring, key_len);
+
+hash_offset_entry = CDB_HASH_ENTRY * (key_hash & CDB_HASH_MASK);
+hash_offset = cdb_unpack(cdbp->cdb_offsets + hash_offset_entry);
+hash_offlen = cdb_unpack(cdbp->cdb_offsets + hash_offset_entry + 4);
+
+/* If the offset length is zero this key cannot be in the file */
+
+if (hash_offlen == 0)
+  return FAIL;
+
+hash_slotnm = (key_hash >> 8) % hash_offlen;
+
+/* check to ensure that the file is not corrupt
+ * if the hash_offset + (hash_offlen * CDB_HASH_ENTRY) is longer
+ * than the file, then we have problems.... */
+
+if ((hash_offset + (hash_offlen * CDB_HASH_ENTRY)) > cdbp->filelen)
+  {
+  *errmsg = string_sprintf("cdb: corrupt cdb file %s (too short)",
+		      filename);
+  DEBUG(D_lookup) debug_printf("%s\n", *errmsg);
+  return DEFER;
   }
 
-  cur_offset = hash_offset + (hash_slotnm * CDB_HASH_ENTRY);
-  end_offset = hash_offset + (hash_offlen * CDB_HASH_ENTRY);
-  /* if we are allowed to we use mmap here.... */
+cur_offset = hash_offset + (hash_slotnm * CDB_HASH_ENTRY);
+end_offset = hash_offset + (hash_offlen * CDB_HASH_ENTRY);
+
+/* if we are allowed to we use mmap here.... */
+
 #ifdef HAVE_MMAP
-  /* make sure the mmap was OK */
-  if (cdbp->cdb_map != NULL) {
-    uschar * cur_pos = cur_offset + cdbp->cdb_map;
-    uschar * end_pos = end_offset + cdbp->cdb_map;
-    for (loop = 0; (loop < hash_offlen); ++loop) {
-      item_hash = cdb_unpack(cur_pos);
-      cur_pos += 4;
-      item_posn = cdb_unpack(cur_pos);
-      cur_pos += 4;
-      /* if the position is zero then we have a definite miss */
-      if (item_posn == 0)
-       return FAIL;
-
-      if (item_hash == key_hash) {
-       /* matching hash value */
-       uschar * item_ptr = cdbp->cdb_map + item_posn;
-       item_key_len = cdb_unpack(item_ptr);
-       item_ptr += 4;
-       item_dat_len = cdb_unpack(item_ptr);
-       item_ptr += 4;
-       /* check key length matches */
-       if (item_key_len == key_len) {
-         /* finally check if key matches */
-         if (Ustrncmp(keystring, item_ptr, key_len) == 0) {
-           /* we have a match....
-            * make item_ptr point to data */
-           item_ptr += item_key_len;
-           /* ... and the returned result */
-           *result = store_get(item_dat_len + 1);
-           memcpy(*result, item_ptr, item_dat_len);
-           (*result)[item_dat_len] = 0;
-           return OK;
-         }
-       }
-      }
-      /* handle warp round of table */
-      if (cur_pos == end_pos)
-       cur_pos = cdbp->cdb_map + hash_offset;
-    }
-    /* looks like we failed... */
-    return FAIL;
-  }
-#endif /* HAVE_MMAP */
-  for (loop = 0; (loop < hash_offlen); ++loop) {
-    uschar packbuf[8];
-    if (lseek(cdbp->fileno, (off_t) cur_offset,SEEK_SET) == -1) return DEFER;
-    if (cdb_bread(cdbp->fileno, packbuf,8) == -1) return DEFER;
-    item_hash = cdb_unpack(packbuf);
-    item_posn = cdb_unpack(packbuf + 4);
+/* make sure the mmap was OK */
+if (cdbp->cdb_map != NULL)
+  {
+  uschar * cur_pos = cur_offset + cdbp->cdb_map;
+  uschar * end_pos = end_offset + cdbp->cdb_map;
+
+  for (loop = 0; (loop < hash_offlen); ++loop)
+    {
+    item_hash = cdb_unpack(cur_pos);
+    cur_pos += 4;
+    item_posn = cdb_unpack(cur_pos);
+    cur_pos += 4;
+
     /* if the position is zero then we have a definite miss */
+
     if (item_posn == 0)
       return FAIL;
 
-    if (item_hash == key_hash) {
-      /* matching hash value */
-      if (lseek(cdbp->fileno, (off_t) item_posn, SEEK_SET) == -1) return DEFER;
-      if (cdb_bread(cdbp->fileno, packbuf, 8) == -1) return DEFER;
-      item_key_len = cdb_unpack(packbuf);
+    if (item_hash == key_hash)
+      {					/* matching hash value */
+      uschar * item_ptr = cdbp->cdb_map + item_posn;
+
+      item_key_len = cdb_unpack(item_ptr);
+      item_ptr += 4;
+      item_dat_len = cdb_unpack(item_ptr);
+      item_ptr += 4;
+
       /* check key length matches */
-      if (item_key_len == key_len) {
-       /* finally check if key matches */
-       uschar * item_key = store_get(key_len);
-       if (cdb_bread(cdbp->fileno, item_key, key_len) == -1) return DEFER;
-       if (Ustrncmp(keystring, item_key, key_len) == 0) {
-         /* Reclaim some store */
-         store_reset(item_key);
-         /* matches - get data length */
-         item_dat_len = cdb_unpack(packbuf + 4);
-         /* then we build a new result string */
-         *result = store_get(item_dat_len + 1);
-         if (cdb_bread(cdbp->fileno, *result, item_dat_len) == -1)
-           return DEFER;
-         (*result)[item_dat_len] = 0;
-         return OK;
-       }
+
+      if (item_key_len == key_len)
+	{
+	 /* finally check if key matches */
+	 if (Ustrncmp(keystring, item_ptr, key_len) == 0)
+	   {
+	   /* we have a match....  * make item_ptr point to data */
+
+	   item_ptr += item_key_len;
+
+	   /* ... and the returned result */
+
+	   *result = store_get(item_dat_len + 1);
+	   memcpy(*result, item_ptr, item_dat_len);
+	   (*result)[item_dat_len] = 0;
+	   return OK;
+	   }
+	}
+      }
+    /* handle warp round of table */
+    if (cur_pos == end_pos)
+    cur_pos = cdbp->cdb_map + hash_offset;
+    }
+  /* looks like we failed... */
+  return FAIL;
+  }
+
+#endif /* HAVE_MMAP */
+
+for (loop = 0; (loop < hash_offlen); ++loop)
+  {
+  uschar packbuf[8];
+
+  if (lseek(cdbp->fileno, (off_t) cur_offset,SEEK_SET) == -1) return DEFER;
+  if (cdb_bread(cdbp->fileno, packbuf,8) == -1) return DEFER;
+
+  item_hash = cdb_unpack(packbuf);
+  item_posn = cdb_unpack(packbuf + 4);
+
+  /* if the position is zero then we have a definite miss */
+
+  if (item_posn == 0)
+    return FAIL;
+
+  if (item_hash == key_hash)
+    {						/* matching hash value */
+    if (lseek(cdbp->fileno, (off_t) item_posn, SEEK_SET) == -1) return DEFER;
+    if (cdb_bread(cdbp->fileno, packbuf, 8) == -1) return DEFER;
+
+    item_key_len = cdb_unpack(packbuf);
+
+    /* check key length matches */
+
+    if (item_key_len == key_len)
+      {					/* finally check if key matches */
+      uschar * item_key = store_get(key_len);
+
+      if (cdb_bread(cdbp->fileno, item_key, key_len) == -1) return DEFER;
+      if (Ustrncmp(keystring, item_key, key_len) == 0) {
+
        /* Reclaim some store */
        store_reset(item_key);
+
+       /* matches - get data length */
+       item_dat_len = cdb_unpack(packbuf + 4);
+
+       /* then we build a new result string.  We know we have enough
+       memory so disable Coverity errors about the tainted item_dat_ken */
+
+       *result = store_get(item_dat_len + 1);
+       /* coverity[tainted_data] */
+       if (cdb_bread(cdbp->fileno, *result, item_dat_len) == -1)
+	 return DEFER;
+
+       /* coverity[tainted_data] */
+       (*result)[item_dat_len] = 0;
+       return OK;
+      }
+      /* Reclaim some store */
+      store_reset(item_key);
       }
     }
-    cur_offset += 8;
+  cur_offset += 8;
 
-    /* handle warp round of table */
-    if (cur_offset == end_offset)
-      cur_offset = hash_offset;
+  /* handle warp round of table */
+  if (cur_offset == end_offset)
+  cur_offset = hash_offset;
   }
-  return FAIL;
+return FAIL;
 }
 
 
diff --git a/src/src/rda.c b/src/src/rda.c
index 476f06050..5df361e31 100644
--- a/src/src/rda.c
+++ b/src/src/rda.c
@@ -474,11 +474,12 @@ rda_read_string(int fd, uschar **sp)
 int len;
 
 if (read(fd, &len, sizeof(int)) != sizeof(int)) return FALSE;
-if (len == 0) *sp = NULL; else
-  {
-  *sp = store_get(len);
-  if (read(fd, *sp, len) != len) return FALSE;
-  }
+if (len == 0)
+  *sp = NULL;
+else
+  /* We know we have enough memory so disable the error on "len" */
+  /* coverity[tainted_data] */
+  if (read(fd, *sp = store_get(len), len) != len) return FALSE;
 return TRUE;
 }
 
diff --git a/src/src/spool_in.c b/src/src/spool_in.c
index 1dcae49bd..e1d6e3422 100644
--- a/src/src/spool_in.c
+++ b/src/src/spool_in.c
@@ -672,10 +672,12 @@ DEBUG(D_deliver)
 #endif  /* COMPILE_UTILITY */
 
 /* After reading the tree, the next line has not yet been read into the
-buffer. It contains the count of recipients which follow on separate lines. */
+buffer. It contains the count of recipients which follow on separate lines.
+Apply an arbitrary sanity check.*/
 
 if (Ufgets(big_buffer, big_buffer_size, f) == NULL) goto SPOOL_READ_ERROR;
-if (sscanf(CS big_buffer, "%d", &rcount) != 1) goto SPOOL_FORMAT_ERROR;
+if (sscanf(CS big_buffer, "%d", &rcount) != 1 || rcount > 16384)
+  goto SPOOL_FORMAT_ERROR;
 
 #ifndef COMPILE_UTILITY
 DEBUG(D_deliver) debug_printf("recipients_count=%d\n", rcount);
@@ -684,6 +686,10 @@ DEBUG(D_deliver) debug_printf("recipients_count=%d\n", rcount);
 recipients_list_max = rcount;
 recipients_list = store_get(rcount * sizeof(recipient_item));
 
+/* We sanitised the count and know we have enough memory, so disable
+the Coverity error on recipients_count */
+/* coverity[tainted_data] */
+
 for (recipients_count = 0; recipients_count < rcount; recipients_count++)
   {
   int nn;
diff --git a/src/src/transports/queuefile.c b/src/src/transports/queuefile.c
index 25747b3ab..7f10706c1 100644
--- a/src/src/transports/queuefile.c
+++ b/src/src/transports/queuefile.c
@@ -116,29 +116,29 @@ if (link_file)
   op = US"linking";
   s = dstpath;
   }
+else					/* use data copy */
+  {
+  DEBUG(D_transport) debug_printf("%s transport, copying %s => %s\n",
+    tb->name, srcpath, dstpath);
 
-/* use data copy */
-
-DEBUG(D_transport) debug_printf("%s transport, copying %s => %s\n",
-  tb->name, srcpath, dstpath);
-
-if (  (s = dstpath,
-       (dstfd = openat(ddfd, CCS filename, O_RDWR|O_CREAT|O_EXCL, SPOOL_MODE))
-       < 0
-      )
-   ||    is_hdr_file
-      && (s = srcpath, (srcfd = openat(sdfd, CCS filename, O_RDONLY)) < 0)
-   )
-  op = US"opening";
+  if (  (s = dstpath,
+	 (dstfd = openat(ddfd, CCS filename, O_RDWR|O_CREAT|O_EXCL, SPOOL_MODE))
+	 < 0
+	)
+     ||    is_hdr_file
+	&& (s = srcpath, (srcfd = openat(sdfd, CCS filename, O_RDONLY)) < 0)
+     )
+    op = US"opening";
 
-else
-  if (s = dstpath, fchmod(dstfd, SPOOL_MODE) != 0)
-    op = US"setting perms on";
   else
-    if (!copy_spool_file(dstfd, srcfd))
-      op = US"creating";
+    if (s = dstpath, fchmod(dstfd, SPOOL_MODE) != 0)
+      op = US"setting perms on";
     else
-      return TRUE;
+      if (!copy_spool_file(dstfd, srcfd))
+	op = US"creating";
+      else
+	return TRUE;
+  }
 
 addr->basic_errno = errno;
 addr->message = string_sprintf("%s transport %s file: %s failed with error: %s",