From: Kevin McCarthy <kevin@8t8.us>
Date: Mon, 2 Sep 2019 02:10:23 +0000 (-0700)
Subject: Fix memory leak when attaching messages.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=c28cba770ae6a053d7bebe5bdd5c94a31896d935;p=mutt

Fix memory leak when attaching messages.

Setting actx->idx[]->content->parts to NULL causes a memory leak,
because message attachments store a BODY there that needs to be freed.

I looked through the various use cases of ci_send_message() and
actually believe content->parts will be NULL for all other cases
except message attachments.  From the comment added to
delete_attachment():

  Other ci_send_message() message constructors are careful to free
  any body->parts, removing depth:
   - mutt_prepare_template() used by postponed, resent, and draft files
   - mutt_copy_body() used by the recvattach menu and $forward_attachments.

  I believe it is safe to completely remove the "content->parts =
  NULL" statement.  But for safety, am doing so only for the case
  it must be avoided: message attachments.
---

diff --git a/compose.c b/compose.c
index 48765352..c64245b0 100644
--- a/compose.c
+++ b/compose.c
@@ -604,7 +604,22 @@ static int delete_attachment (ATTACH_CONTEXT *actx, int x)
   }
 
   idx[rindex]->content->next = NULL;
-  idx[rindex]->content->parts = NULL;
+  /* mutt_make_message_attach() creates body->parts, shared by
+   * body->hdr->content.  If we NULL out that, it creates a memory
+   * leak because mutt_free_body() frees body->parts, not
+   * body->hdr->content.
+   *
+   * Other ci_send_message() message constructors are careful to free
+   * any body->parts, removing depth:
+   *  - mutt_prepare_template() used by postponed, resent, and draft files
+   *  - mutt_copy_body() used by the recvattach menu and $forward_attachments.
+   *
+   * I believe it is safe to completely remove the "content->parts =
+   * NULL" statement.  But for safety, am doing so only for the case
+   * it must be avoided: message attachments.
+   */
+  if (!idx[rindex]->content->hdr)
+    idx[rindex]->content->parts = NULL;
   mutt_free_body (&(idx[rindex]->content));
   FREE (&idx[rindex]->tree);
   FREE (&idx[rindex]);
@@ -1503,7 +1518,9 @@ int mutt_compose_menu (HEADER *msg,   /* structure for new message */
             {
               /* avoid freeing other attachments */
               actx->idx[i]->content->next = NULL;
-              actx->idx[i]->content->parts = NULL;
+              /* See the comment in delete_attachment() */
+              if (!actx->idx[i]->content->hdr)
+                actx->idx[i]->content->parts = NULL;
               mutt_free_body (&actx->idx[i]->content);
             }
           }