[exim-cvs] Fix unsigned < 0 check

Top Page
Delete this message
Reply to this message
Author: Exim Git Commits Mailing List
Date:  
To: exim-cvs
Subject: [exim-cvs] Fix unsigned < 0 check
Gitweb: http://git.exim.org/exim.git/commitdiff/cb54b2a05b5f5f3548ac98e74b90eb8633052919
Commit:     cb54b2a05b5f5f3548ac98e74b90eb8633052919
Parent:     66be95e02b2ba6a834a6dbee16061176ad85019a
Author:     Phil Pennock <pdp@???>
AuthorDate: Mon Jul 14 02:59:52 2014 -0400
Committer:  Phil Pennock <pdp@???>
CommitDate: Mon Jul 14 02:59:52 2014 -0400


    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 files changed, 28 insertions(+), 10 deletions(-)


diff --git a/src/src/malware.c b/src/src/malware.c
index 7685554..04b5887 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);