From cb54b2a05b5f5f3548ac98e74b90eb8633052919 Mon Sep 17 00:00:00 2001 From: Phil Pennock Date: Mon, 14 Jul 2014 02:59:52 -0400 Subject: [PATCH] Fix unsigned < 0 check Two places in malware.c were using `fsize`, defined as `unsigned int`, to receive the result of `lseek()` and then checking if the value was less than 0. As clang says: ``` malware.c:1228:46: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare] if ((fsize = lseek(clam_fd, 0, SEEK_END)) < 0) { ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~ ``` Fix. Use `off_t`, which we're already using elsewhere, then use `fsize_uint` to handle off_t being potentially 64-bit, and a sanity-check on conversion which hopefully won't be optimised away by compilers. --- src/src/malware.c | 38 ++++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/src/src/malware.c b/src/src/malware.c index 7685554ae..04b5887e7 100644 --- a/src/src/malware.c +++ b/src/src/malware.c @@ -456,7 +456,8 @@ malware_internal(uschar **listptr, uschar *eml_filename, BOOL faking) /* v0.0 - initial release -- support for unix sockets */ { int result; - unsigned int fsize; + off_t fsize; + unsigned int fsize_uint; uschar * tmpbuf, *drweb_fbuf; int drweb_rc, drweb_cmd, drweb_flags = 0x0000, drweb_fd, drweb_vnum, drweb_slen, drweb_fin = 0x0000; @@ -484,6 +485,14 @@ malware_internal(uschar **listptr, uschar *eml_filename, BOOL faking) eml_filename, strerror(err)), sock); } + fsize_uint = (unsigned int) fsize; + if ((off_t)fsize_uint != fsize) { + (void)close(drweb_fd); + return m_errlog_defer_3(scanent, + string_sprintf("seeking spool file %s, size overflow", + eml_filename), + sock); + } drweb_slen = htonl(fsize); lseek(drweb_fd, 0, SEEK_SET); @@ -501,11 +510,11 @@ malware_internal(uschar **listptr, uschar *eml_filename, BOOL faking) sock); } - if (!(drweb_fbuf = (uschar *) malloc (fsize))) { + if (!(drweb_fbuf = (uschar *) malloc (fsize_uint))) { (void)close(drweb_fd); return m_errlog_defer_3(scanent, string_sprintf("unable to allocate memory %u for file (%s)", - fsize, eml_filename), + fsize_uint, eml_filename), sock); } @@ -1035,7 +1044,8 @@ malware_internal(uschar **listptr, uschar *eml_filename, BOOL faking) host_item connhost; uschar *clamav_fbuf; int clam_fd, result; - unsigned int fsize; + off_t fsize; + unsigned int fsize_uint; BOOL use_scan_command = FALSE; clamd_address_container * clamd_address_vector[MAX_CLAMD_SERVERS]; int current_server; @@ -1231,17 +1241,25 @@ malware_internal(uschar **listptr, uschar *eml_filename, BOOL faking) eml_filename, strerror(err)), sock); } + fsize_uint = (unsigned int) fsize; + if ((off_t)fsize_uint != fsize) { + CLOSE_SOCKDATA; (void)close(clam_fd); + return m_errlog_defer_3(scanent, + string_sprintf("seeking spool file %s, size overflow", + eml_filename), + sock); + } lseek(clam_fd, 0, SEEK_SET); - if (!(clamav_fbuf = (uschar *) malloc (fsize))) { + if (!(clamav_fbuf = (uschar *) malloc (fsize_uint))) { CLOSE_SOCKDATA; (void)close(clam_fd); return m_errlog_defer_3(scanent, string_sprintf("unable to allocate memory %u for file (%s)", - fsize, eml_filename), + fsize_uint, eml_filename), sock); } - if ((result = read(clam_fd, clamav_fbuf, fsize)) < 0) { + if ((result = read(clam_fd, clamav_fbuf, fsize_uint)) < 0) { int err = errno; free(clamav_fbuf); CLOSE_SOCKDATA; (void)close(clam_fd); return m_errlog_defer_3(scanent, @@ -1253,7 +1271,7 @@ malware_internal(uschar **listptr, uschar *eml_filename, BOOL faking) /* send file body to socket */ #ifdef WITH_OLD_CLAMAV_STREAM - if (send(sockData, clamav_fbuf, fsize, 0) < 0) { + if (send(sockData, clamav_fbuf, fsize_uint, 0) < 0) { free(clamav_fbuf); CLOSE_SOCKDATA; return m_errlog_defer_3(scanent, string_sprintf("unable to send file body to socket (%s:%u)", @@ -1261,10 +1279,10 @@ malware_internal(uschar **listptr, uschar *eml_filename, BOOL faking) sock); } #else - send_size = htonl(fsize); + send_size = htonl(fsize_uint); send_final_zeroblock = 0; if ((send(sock, &send_size, sizeof(send_size), 0) < 0) || - (send(sock, clamav_fbuf, fsize, 0) < 0) || + (send(sock, clamav_fbuf, fsize_uint, 0) < 0) || (send(sock, &send_final_zeroblock, sizeof(send_final_zeroblock), 0) < 0)) { free(clamav_fbuf); -- 2.25.1