From d315eda12f25ca2f72ca56b777a427c9ee7188e1 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Sat, 29 Apr 2017 13:28:38 +0100 Subject: [PATCH] tidying: coverity fixes --- src/src/dkim.c | 11 +++++----- src/src/dkim_transport.c | 20 +++++++++++------- src/src/expand.c | 4 ++-- src/src/functions.h | 1 - src/src/lookups/cdb.c | 4 ++-- src/src/malware.c | 12 +++++++---- src/src/receive.c | 1 + src/src/transport.c | 8 ++++---- src/src/transports/appendfile.c | 2 +- src/src/transports/autoreply.c | 36 +++++++++++++++++---------------- src/src/transports/tf_maildir.c | 10 +++++---- 11 files changed, 62 insertions(+), 47 deletions(-) diff --git a/src/src/dkim.c b/src/src/dkim.c index f0dfb8af3..b9dbce68d 100644 --- a/src/src/dkim.c +++ b/src/src/dkim.c @@ -625,11 +625,12 @@ while ((dkim_signing_domain = string_nextinlist(&dkim_domain, &sep, NULL, 0))) if (prefix) pdkim_feed(ctx, prefix, Ustrlen(prefix)); - lseek(fd, off, SEEK_SET); - - while ((sread = read(fd, &buf, sizeof(buf))) > 0) - if ((pdkim_rc = pdkim_feed(ctx, buf, sread)) != PDKIM_OK) - goto pk_bad; + if (lseek(fd, off, SEEK_SET) < 0) + sread = -1; + else + while ((sread = read(fd, &buf, sizeof(buf))) > 0) + if ((pdkim_rc = pdkim_feed(ctx, buf, sread)) != PDKIM_OK) + goto pk_bad; /* Handle failed read above. */ if (sread == -1) diff --git a/src/src/dkim_transport.c b/src/src/dkim_transport.c index 4538b36e3..a654c2516 100644 --- a/src/src/dkim_transport.c +++ b/src/src/dkim_transport.c @@ -38,10 +38,12 @@ if (dkim->dkim_strict) return TRUE; } +/* Send the file at in_fd down the output fd */ + static BOOL dkt_send_file(int out_fd, int in_fd, off_t off, size_t size) { -DEBUG(D_transport) debug_printf("send file fd=%d size=%d\n", out_fd, size - off); +DEBUG(D_transport) debug_printf("send file fd=%d size=%l\n", out_fd, size - off); /*XXX should implement timeout, like transport_write_block_fd() ? */ @@ -67,10 +69,10 @@ else int sread, wwritten; /* Rewind file */ - lseek(in_fd, off, SEEK_SET); + if (lseek(in_fd, off, SEEK_SET) < 0) return FALSE; /* Send file down the original fd */ - while((sread = read(in_fd, deliver_out_buffer, DELIVER_OUT_BUFFER_SIZE)) >0) + while((sread = read(in_fd, deliver_out_buffer, DELIVER_OUT_BUFFER_SIZE)) > 0) { uschar * p = deliver_out_buffer; /* write the chunk */ @@ -120,7 +122,7 @@ int save_fd = tctx->u.fd; int save_options = tctx->options; BOOL save_wireformat = spool_file_wireformat; uschar * hdrs, * dkim_signature; -int siglen, hsize; +int siglen = 0, hsize; const uschar * errstr; BOOL rc; @@ -164,7 +166,7 @@ temporarily set the marker for possible already-CRLF input. */ tctx->options &= ~topt_escape_headers; spool_file_wireformat = TRUE; transport_write_reset(0); -if ( !write_chunk(tctx, dkim_signature, siglen) +if ( siglen > 0 && !write_chunk(tctx, dkim_signature, siglen) || !write_chunk(tctx, hdrs, hsize)) return FALSE; @@ -256,7 +258,11 @@ else if (!(rc = dkt_sign_fail(dkim, &save_errno))) #ifndef HAVE_LINUX_SENDFILE if (options & topt_use_bdat) #endif - k_file_size = lseek(dkim_fd, 0, SEEK_END); /* Fetch file size */ + if ((k_file_size = lseek(dkim_fd, 0, SEEK_END)) < 0) + { + *err = string_sprintf("dkim spoolfile seek: %s", strerror(errno)); + goto CLEANUP; + } if (options & topt_use_bdat) { @@ -292,7 +298,7 @@ if (!dkt_send_file(tctx->u.fd, dkim_fd, 0, k_file_size)) CLEANUP: /* unlink -K file */ - (void)close(dkim_fd); + if (dkim_fd >= 0) (void)close(dkim_fd); Uunlink(dkim_spool_name); errno = save_errno; return rc; diff --git a/src/src/expand.c b/src/src/expand.c index 1753aa307..a68849dc3 100644 --- a/src/src/expand.c +++ b/src/src/expand.c @@ -1832,7 +1832,7 @@ switch (vp->type) case vtype_msgbody: /* Pointer to msgbody string */ case vtype_msgbody_end: /* Ditto, the end of the msg */ ss = (uschar **)(val); - if (*ss == NULL && deliver_datafile >= 0) /* Read body when needed */ + if (!*ss && deliver_datafile >= 0) /* Read body when needed */ { uschar *body; off_t start_offset = SPOOL_DATA_START_OFFSET; @@ -1865,7 +1865,7 @@ switch (vp->type) { if (body[--len] == '\n' || body[len] == 0) body[len] = ' '; } } } - return (*ss == NULL)? US"" : *ss; + return *ss ? *ss : US""; case vtype_todbsdin: /* BSD inbox time of day */ return tod_stamp(tod_bsdin); diff --git a/src/src/functions.h b/src/src/functions.h index ee17e9c27..201111623 100644 --- a/src/src/functions.h +++ b/src/src/functions.h @@ -231,7 +231,6 @@ extern uschar *imap_utf7_encode(uschar *, const uschar *, uschar, uschar *, uschar **); extern void invert_address(uschar *, uschar *); -extern BOOL internal_transport_write_message(transport_ctx *, int); extern int ip_addr(void *, int, const uschar *, int); extern int ip_bind(int, int, uschar *, int); extern int ip_connect(int, int, const uschar *, int, int, BOOL); diff --git a/src/src/lookups/cdb.c b/src/src/lookups/cdb.c index bc610467c..153fcf24f 100644 --- a/src/src/lookups/cdb.c +++ b/src/src/lookups/cdb.c @@ -390,8 +390,8 @@ 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; + 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); diff --git a/src/src/malware.c b/src/src/malware.c index e995f47b4..94a271b47 100644 --- a/src/src/malware.c +++ b/src/src/malware.c @@ -605,7 +605,8 @@ if (!malware_ok) if ((fsize = lseek(drweb_fd, 0, SEEK_END)) == -1) { - int err = errno; + int err; +badseek: err = errno; (void)close(drweb_fd); return m_errlog_defer_3(scanent, NULL, string_sprintf("can't seek spool file %s: %s", @@ -622,7 +623,8 @@ if (!malware_ok) sock); } drweb_slen = htonl(fsize); - lseek(drweb_fd, 0, SEEK_SET); + if (lseek(drweb_fd, 0, SEEK_SET) < 0) + goto badseek; DEBUG(D_acl) debug_printf_indent("Malware scan: issuing %s remote scan [%s]\n", scanner_name, scanner_options); @@ -1478,7 +1480,8 @@ if (!malware_ok) } if ((fsize = lseek(clam_fd, 0, SEEK_END)) < 0) { - int err = errno; + int err; +b_seek: err = errno; CLOSE_SOCKDATA; (void)close(clam_fd); return m_errlog_defer_3(scanent, NULL, string_sprintf("can't seek spool file %s: %s", @@ -1494,7 +1497,8 @@ if (!malware_ok) eml_filename), sock); } - lseek(clam_fd, 0, SEEK_SET); + if (lseek(clam_fd, 0, SEEK_SET) < 0) + goto b_seek; if (!(clamav_fbuf = US malloc(fsize_uint))) { diff --git a/src/src/receive.c b/src/src/receive.c index 3d92a8479..b03ab71ed 100644 --- a/src/src/receive.c +++ b/src/src/receive.c @@ -3676,6 +3676,7 @@ dcc_ok = 0; version supplied with Exim always accepts, but this is a hook for sysadmins to supply their own checking code. The local_scan() function is run even when all the recipients have been discarded. */ +/*XXS could we avoid this for the standard case, given that few people will use it? */ lseek(data_fd, (long int)SPOOL_DATA_START_OFFSET, SEEK_SET); diff --git a/src/src/transport.c b/src/src/transport.c index 7806e3957..71fd9dac2 100644 --- a/src/src/transport.c +++ b/src/src/transport.c @@ -473,7 +473,7 @@ for (ptr = start; ptr < end; ptr++) /* If CHUNKING, prefix with BDAT (size) NON-LAST. Also, reap responses from previous SMTP commands. */ - if (tctx && tctx->options & topt_use_bdat && tctx->chunk_cb) + if (tctx->options & topt_use_bdat && tctx->chunk_cb) { if ( tctx->chunk_cb(tctx, (unsigned)len, 0) != OK || !transport_write_block(tctx, deliver_out_buffer, len, FALSE) @@ -490,7 +490,7 @@ for (ptr = start; ptr < end; ptr++) /* Remove CR before NL if required */ if ( *ptr == '\r' && ptr[1] == '\n' - && (!tctx || !(tctx->options & topt_use_crlf)) + && !(tctx->options & topt_use_crlf) && spool_file_wireformat ) ptr++; @@ -501,7 +501,7 @@ for (ptr = start; ptr < end; ptr++) /* Insert CR before NL if required */ - if (tctx && tctx->options & topt_use_crlf && !spool_file_wireformat) + if (tctx->options & topt_use_crlf && !spool_file_wireformat) *chunk_ptr++ = '\r'; *chunk_ptr++ = '\n'; transport_newlines++; @@ -898,7 +898,7 @@ Returns: TRUE on success; FALSE (with errno) on failure. is incremented by the number of bytes written. */ -BOOL +static BOOL internal_transport_write_message(transport_ctx * tctx, int size_limit) { int len, size = 0; diff --git a/src/src/transports/appendfile.c b/src/src/transports/appendfile.c index 760e96039..da5d8aa0a 100644 --- a/src/src/transports/appendfile.c +++ b/src/src/transports/appendfile.c @@ -949,7 +949,7 @@ used = Ustrlen(deliver_out_buffer); /* Rewind the temporary file, and copy it over in chunks. */ -lseek(from_fd, 0 , SEEK_SET); +if (lseek(from_fd, 0 , SEEK_SET) < 0) return DEFER; while (size > 0) { diff --git a/src/src/transports/autoreply.c b/src/src/transports/autoreply.c index cdc4bdd05..3b4463075 100644 --- a/src/src/transports/autoreply.c +++ b/src/src/transports/autoreply.c @@ -485,7 +485,7 @@ if (oncelog != NULL && *oncelog != 0 && to != NULL) { EXIM_DATUM key_datum, result_datum; EXIM_DBOPEN(oncelog, O_RDWR|O_CREAT, ob->mode, &dbm_file); - if (dbm_file == NULL) + if (!dbm_file) { addr->transport_return = DEFER; addr->message = string_sprintf("Failed to open %s file %s when sending " @@ -745,30 +745,32 @@ if (cache_fd >= 0) { uschar *from = cache_buff; int size = cache_size; - (void)lseek(cache_fd, 0, SEEK_SET); - if (cache_time == NULL) + if (lseek(cache_fd, 0, SEEK_SET) == 0) { - cache_time = from + size; - memcpy(cache_time + sizeof(time_t), to, add_size - sizeof(time_t)); - size += add_size; - - if (cache_size > 0 && size > ob->once_file_size) + if (!cache_time) { - from += sizeof(time_t) + Ustrlen(from + sizeof(time_t)) + 1; - size -= (from - cache_buff); + cache_time = from + size; + memcpy(cache_time + sizeof(time_t), to, add_size - sizeof(time_t)); + size += add_size; + + if (cache_size > 0 && size > ob->once_file_size) + { + from += sizeof(time_t) + Ustrlen(from + sizeof(time_t)) + 1; + size -= (from - cache_buff); + } } - } - memcpy(cache_time, &now, sizeof(time_t)); - if(write(cache_fd, from, size) != size) - DEBUG(D_transport) debug_printf("Problem writing cache file %s for %s " - "transport\n", oncelog, tblock->name); + memcpy(cache_time, &now, sizeof(time_t)); + if(write(cache_fd, from, size) != size) + DEBUG(D_transport) debug_printf("Problem writing cache file %s for %s " + "transport\n", oncelog, tblock->name); + } } /* Update DBM file */ -else if (dbm_file != NULL) +else if (dbm_file) { EXIM_DATUM key_datum, value_datum; EXIM_DATUM_INIT(key_datum); /* Some DBM libraries need to have */ @@ -869,7 +871,7 @@ if (logfile != NULL) } END_OFF: -if (dbm_file != NULL) EXIM_DBCLOSE(dbm_file); +if (dbm_file) EXIM_DBCLOSE(dbm_file); if (cache_fd > 0) (void)close(cache_fd); DEBUG(D_transport) debug_printf("%s transport succeeded\n", tblock->name); diff --git a/src/src/transports/tf_maildir.c b/src/src/transports/tf_maildir.c index 7be72896a..9e18a804b 100644 --- a/src/src/transports/tf_maildir.c +++ b/src/src/transports/tf_maildir.c @@ -211,10 +211,12 @@ int len; uschar buffer[256]; sprintf(CS buffer, "%d 1\n", size); len = Ustrlen(buffer); -(void)lseek(fd, 0, SEEK_END); -len = write(fd, buffer, len); -DEBUG(D_transport) - debug_printf("added '%.*s' to maildirsize file\n", len-1, buffer); +if (lseek(fd, 0, SEEK_END) >= 0) + { + len = write(fd, buffer, len); + DEBUG(D_transport) + debug_printf("added '%.*s' to maildirsize file\n", len-1, buffer); + } } -- 2.25.1