Bug#343836: Security: DoS attack possible - crashes on empty message

[Martin Schulze]

Matthias Andree wrote:
> are you using multidrop mode? If so, please test if the attached patch
> fixes the bug.  It is an untested backport from 6.3.1-rc1.
> 
> If you are not using multidrop mode, please provide your configuration
> details (passwords masked!) and a stack backtrace.

Thanks a lot Matthias,

The patch does not apply though, since xfree() is unknown in version 6.2.5.
I assume that the xfree only frees the memory when it is not NULL and sets
the variable to NULL again, so the attached patch should do the same and apply
to the version in Debian sarge/etch/sid.

Regards,

        Joey

-- 
Long noun chains don't automatically imply security.  -- Bruce Schneier

Please always Cc to me when replying to me on the lists.

diff -u fetchmail-6.2.5/transact.c fetchmail-6.2.5/transact.c
--- fetchmail-6.2.5/transact.c
+++ fetchmail-6.2.5/transact.c
@@ -395,10 +395,16 @@
      * at the freed storage and coredump...
      */
     if (msgblk.headers)
+    {
        free(msgblk.headers);
+       msgblk.headers = NULL;
+    }
     free_str_list(&msgblk.recipients);
     if (delivered_to)
+    {
        free(delivered_to);
+       delivered_to = NULL;
+    }
 
     /* initially, no message digest */
     memset(ctl->digest, '\0', sizeof(ctl->digest));
@@ -427,8 +433,6 @@
                if ((n = SockRead(sock, buf, sizeof(buf)-1)) == -1) {
                    set_timeout(0);
                    free(line);
-                   free(msgblk.headers);
-                   msgblk.headers = NULL;
                    return(PS_SOCKET);
                }
                set_timeout(0);
@@ -870,11 +874,7 @@
  process_headers:    
 
     if (retain_mail)
-    {
-       free(msgblk.headers);
-       msgblk.headers = NULL;
        return(PS_RETAINED);
-    }
     if (refuse_mail)
        return(PS_REFUSED);
     /*
@@ -905,7 +905,7 @@
      * to break it in a way that blackholed mail.  Better to pass
      * the occasional duplicate than to do that...
      */
-    if (MULTIDROP(ctl))
+    if (MULTIDROP(ctl) && msgblk.headers)
     {
        MD5_CTX context;
 
@@ -1017,8 +1017,10 @@
       ctl->server.envelope && !strcasecmp(ctl->server.envelope, 
"Delivered-To"))
    {
            find_server_names(delivered_to, ctl, &msgblk.recipients);
+     if (delivered_to) {
        free(delivered_to);
        delivered_to = NULL;
+     }
    }
        else if (received_for)
            /*
@@ -1080,8 +1082,10 @@
        if (outlevel >= O_DEBUG)
            report(stdout,
                   GT_("forwarding and deletion suppressed due to DNS 
errors\n"));
+       if (msgblk.headers) {
        free(msgblk.headers);
        msgblk.headers = NULL;
+       }
        free_str_list(&msgblk.recipients);
        return(PS_TRANSIENT);
     }
@@ -1091,8 +1095,10 @@
        if ((n = open_sink(ctl, &msgblk,
                           &good_addresses, &bad_addresses)) != PS_SUCCESS)
        {
+          if (msgblk.headers) {
            free(msgblk.headers);
            msgblk.headers = NULL;
+         }
            free_str_list(&msgblk.recipients);
            return(n);
        }
@@ -1217,8 +1223,10 @@
     {
        report(stdout, GT_("writing RFC822 msgblk.headers\n"));
        release_sink(ctl);
+       if (msgblk.headers) {
        free(msgblk.headers);
        msgblk.headers = NULL;
+       }
        free_str_list(&msgblk.recipients);
        return(PS_IOERR);
     }