Fix unsigned < 0 check
authorPhil Pennock <pdp@exim.org>
Mon, 14 Jul 2014 06:59:52 +0000 (02:59 -0400)
committerPhil Pennock <pdp@exim.org>
Mon, 14 Jul 2014 06:59:52 +0000 (02:59 -0400)
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

index 7685554..04b5887 100644 (file)
@@ -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);