summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorJeremy Harris <jgh146exb@wizmail.org>2016-10-16 19:28:01 +0100
committerJeremy Harris <jgh146exb@wizmail.org>2016-10-16 20:01:33 +0100
commitd88f0784c1400a06efb1b09d0bfcfa31c284c7d7 (patch)
tree5bd2e5603b8202361720ecbd92c5af315641d62a /src
parent91d08eb383184828de154a118152a334e74f7668 (diff)
Tidying: coverity issues
Diffstat (limited to 'src')
-rw-r--r--src/src/auths/auth-spa.c8
-rw-r--r--src/src/deliver.c25
-rw-r--r--src/src/exim_dbmbuild.c8
-rw-r--r--src/src/lookups/cdb.c266
-rw-r--r--src/src/rda.c11
-rw-r--r--src/src/spool_in.c10
-rw-r--r--src/src/transports/queuefile.c38
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",