On 2010-08-29 at 18:15 +0100, John Horne wrote:
> On Sun, 2010-08-29 at 01:20 -0400, Phil Pennock wrote:
> > Clearly the parse logic in Exim needs to be a little more careful. I'm
> > apparently in the middle of developing a head-cold which is messing up
> > my ability to think clearly, so don't feel that any code I wrote now
> > would be worth the electrons used to encode it. So, patches welcome.
> I've had a quick look at the code, and I think it should be possible to
> cater for this case. I'll bugzilla the problem, and submit a patch if I
> can devise one.
Not seeing a bugzilla mail and I'm #@$%ed if I can figure out (again)
how to get a list of all open bugs from that software. Sometimes I can,
sometimes it wins.
Here's my patch against malware.c which makes the ClamAV response
handling rather more robust. Let me know how it works for you?
-Phil
diff --git a/src/src/malware.c b/src/src/malware.c
index 6e8b3f3..eb0c33c 100644
--- 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 *p,*vname;
+ uschar *p, *vname, *result_tag, *response_end;
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;
+ /* 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 != '/') {
@@ -1615,10 +1624,25 @@ static int malware_internal(uschar **listptr, uschar *eml_filename, BOOL faking)
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,
@@ -1626,50 +1650,76 @@ static int malware_internal(uschar **listptr, uschar *eml_filename, BOOL faking)
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;
- if( *p == '\n' ) *p = '\0';
+ if (*p == '\n') *p = '\0';
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? */
- if((p = Ustrrchr(av_buffer,':')) == NULL) {
+ if((p = Ustrchr(av_buffer,':')) == NULL) {
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 */
- ++p;
- while (*p == ' ') ++p;
+ while (*p && isspace(*++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 */
+
/* ----------------------------------------------------------------------- */
