Rework clamd response handling to be more robust.

In particular, clamd's ExtendedDetectionInfo option broke our parsing.

diff --git a/src/src/malware.c b/src/src/malware.c
index 6e8b3f36dedee69c67af7f4424021052ec62053e..eb0c33cea1efb11aa853129e6ab26ffbae57ad7d 100644 (file)
--- a/src/src/malware.c
+++ b/src/src/malware.c
@@ -1298,7 +1298,7 @@ static int malware_internal(uschar **listptr, uschar *eml_filename, BOOL faking)
       uschar *clamd_options;
       uschar clamd_options_buffer[1024];
       uschar clamd_options_default[] = "/tmp/clamd";
       uschar *clamd_options;
       uschar clamd_options_buffer[1024];
       uschar clamd_options_default[] = "/tmp/clamd";

-      uschar *p,*vname;
+      uschar *p, *vname, *result_tag, *response_end;

       struct sockaddr_un server;
       int sock,bread=0;
       unsigned int port;
       struct sockaddr_un server;
       int sock,bread=0;
       unsigned int port;


@@ -1338,6 +1338,15 @@ static int malware_internal(uschar **listptr, uschar *eml_filename, BOOL faking)
       else
         use_scan_command = FALSE;
 
       else
         use_scan_command = FALSE;
 

+      /* See the discussion of response formats below to see why we really don't
+      like colons in filenames when passing filenames to ClamAV. */
+      if (use_scan_command && Ustrchr(eml_filename, ':')) {
+       log_write(0, LOG_MAIN|LOG_PANIC,
+           "malware acl condition: clamd: local/SCAN mode incompatible with" \
+           " : in path to email filename [%s]", eml_filename);
+       return DEFER;
+      }
+

       /* socket does not start with '/' -> network socket */
       if (*clamd_options != '/') {
 
       /* socket does not start with '/' -> network socket */
       if (*clamd_options != '/') {
 


@@ -1615,10 +1624,25 @@ static int malware_internal(uschar **listptr, uschar *eml_filename, BOOL faking)
         return DEFER;
       }
 
         return DEFER;
       }
 

-      /* Check the result. ClamAV Returns
-         infected: -> "<filename>: <virusname> FOUND"
-         not-infected: -> "<filename>: OK"
-         error: -> "<filename>: <errcode> ERROR */
+      /* Check the result. ClamAV returns one of two result formats.
+      In the basic mode, the response is of the form:
+        infected: -> "<filename>: <virusname> FOUND"
+        not-infected: -> "<filename>: OK"
+        error: -> "<filename>: <errcode> ERROR
+      If the ExtendedDetectionInfo option has been turned on, then we get:
+        "<filename>: <virusname>(<virushash>:<virussize>) FOUND"
+      for the infected case.  Compare:
+/tmp/eicar.com: Eicar-Test-Signature FOUND
+/tmp/eicar.com: Eicar-Test-Signature(44d88612fea8a8f36de82e1278abb02f:68) FOUND
+
+      In the streaming case, clamd uses the filename "stream" which you should
+      be able to verify with { ktrace clamdscan --stream /tmp/eicar.com }.  (The
+      client app will replace "stream" with the original filename before returning
+      results to stdout, but the trace shows the data).
+
+      We will assume that the pathname passed to clamd from Exim does not contain
+      a colon.  We will have whined loudly above if the eml_filename does (and we're
+      passing a filename to clamd). */

 
       if (!(*av_buffer)) {
         log_write(0, LOG_MAIN|LOG_PANIC,
 
       if (!(*av_buffer)) {
         log_write(0, LOG_MAIN|LOG_PANIC,


@@ -1626,50 +1650,76 @@ static int malware_internal(uschar **listptr, uschar *eml_filename, BOOL faking)
         return DEFER;
       }
 
         return DEFER;
       }
 

-      /* strip newline at the end (won't be present for zINSTREAM) */
+      /* strip newline at the end (won't be present for zINSTREAM)
+      (also any trailing whitespace, which shouldn't exist, but we depend upon
+      this below, so double-check) */

       p = av_buffer + Ustrlen(av_buffer) - 1;
       p = av_buffer + Ustrlen(av_buffer) - 1;

-      if( *p == '\n' ) *p = '\0';
+      if (*p == '\n') *p = '\0';

 
       DEBUG(D_acl) debug_printf("Malware response: %s\n", av_buffer);
 
 
       DEBUG(D_acl) debug_printf("Malware response: %s\n", av_buffer);
 

+      while (isspace(*--p) && (p > av_buffer))
+       *p = '\0';
+      if (*p) ++p;
+      response_end = p;
+

       /* colon in returned output? */
       /* colon in returned output? */

-      if((p = Ustrrchr(av_buffer,':')) == NULL) {
+      if((p = Ustrchr(av_buffer,':')) == NULL) {

         log_write(0, LOG_MAIN|LOG_PANIC,
         log_write(0, LOG_MAIN|LOG_PANIC,

-                  "malware acl condition: clamd: ClamAV returned malformed result: %s",
+                  "malware acl condition: clamd: ClamAV returned malformed result (missing colon): %s",

                   av_buffer);
         return DEFER;
       }
 
       /* strip filename */
                   av_buffer);
         return DEFER;
       }
 
       /* strip filename */

-      ++p;
-      while (*p == ' ') ++p;
+      while (*p && isspace(*++p)) /**/;

       vname = p;
       vname = p;

-      if ((p = Ustrstr(vname, "FOUND"))!=NULL) {
-           *p=0;
-           for (--p;p>vname && *p<=32;p--) *p=0;
-           for (;*vname==32;vname++);
-           Ustrcpy(malware_name_buffer,vname);
-           malware_name = malware_name_buffer;
-           DEBUG(D_acl) debug_printf("Malware found, name \"%s\"\n", malware_name);
-      }
-      else {
-           if (Ustrstr(vname, "ERROR")!=NULL) {
-              /* ClamAV reports ERROR
-              Find line start */
-              for (;*vname!='\n' && vname>av_buffer; vname--);
-              if (*vname=='\n') vname++;
-
-              log_write(0, LOG_MAIN|LOG_PANIC,
-                     "malware acl condition: clamd: ClamAV returned %s",vname);
-              return DEFER;
-           }
-           else {
-              /* Everything should be OK */
-              malware_name = NULL;
-              DEBUG(D_acl) debug_printf("Malware not found\n");
-           }
+
+      /* It would be bad to encounter a virus with "FOUND" in part of the name,
+      but we should at least be resistant to it. */
+      p = Ustrrchr(vname, ' ');
+      if (p)
+       result_tag = p + 1;
+      else
+       result_tag = vname;
+
+      if (Ustrcmp(result_tag, "FOUND") == 0) {
+       /* p should still be the whitespace before the result_tag */
+       while (isspace(*p)) --p;
+       *++p = '\0';
+        /* Strip off the extended information too, which will be in parens
+        after the virus name, with no intervening whitespace. */
+       if (*--p == ')') {
+         /* "(hash:size)", so previous '(' will do; if not found, we have
+         a curious virus name, but not an error. */
+         p = Ustrrchr(vname, '(');
+         if (p)
+           *p = '\0';
+       }
+       Ustrncpy(malware_name_buffer, vname, sizeof(malware_name_buffer)-1);
+       malware_name = malware_name_buffer;
+       DEBUG(D_acl) debug_printf("Malware found, name \"%s\"\n", malware_name);
+
+      } else if (Ustrcmp(result_tag, "ERROR") == 0) {
+       log_write(0, LOG_MAIN|LOG_PANIC,
+                 "malware acl condition: clamd: ClamAV returned: %s",
+                 av_buffer);
+       return DEFER;
+
+      } else if (Ustrcmp(result_tag, "OK") == 0) {
+       /* Everything should be OK */
+       malware_name = NULL;
+       DEBUG(D_acl) debug_printf("Malware not found\n");
+
+      } else {
+       log_write(0, LOG_MAIN|LOG_PANIC,
+                 "malware acl condition: clamd: unparseable response from ClamAV: {%s}",
+                 av_buffer);
+       return DEFER;

       }
       }

-    }
+
+    } /* clamd */
+

     /* ----------------------------------------------------------------------- */
 
 
     /* ----------------------------------------------------------------------- */