Fix imap sync segfault due to inactive headers during an expunge. (closes #3971)

Mutt has several places where it turns off h->active as a hack.  For
example to avoid FLAG updates, or to exclude from imap_exec_msgset.

Unfortunately, when a reopen is allowed and the IMAP_EXPUNGE_PENDING
flag becomes set (e.g. a flag update to a modified header),
imap_expunge_mailbox() will be called by imap_cmd_finish().

The mx_update_tables() would free and remove these "inactive" headers,
despite that an EXPUNGE was not received for them.  This would result
in memory leaks and segfaults due to dangling pointers in the
msn_index and uid_hash.

There should probably be a more elegant solution, removing the initial
hacks.  However, this is causing a segfault, and the best solution
right now is to turn active back on for non-expunged messages in
imap_expunge_mailbox().

Extra thanks to chdiza, who bravely runs tip and found this issue
quickly.

diff --git a/imap/imap.c b/imap/imap.c
index 28cf6b94368fa6a72dd983a1b8dec5d1d2a38ff3..f10987a142b03b8c59d640b9a5586c4b6901265e 100644 (file)
--- a/imap/imap.c
+++ b/imap/imap.c
@@ -318,7 +318,26 @@ void imap_expunge_mailbox(struct ImapData *idata)
       imap_free_header_data((struct ImapHeaderData **) &h->data);
     }
     else
+    {
       h->index = i;
+      /* Mutt has several places where it turns off h->active as a
+       * hack.  For example to avoid FLAG updates, or to exclude from
+       * imap_exec_msgset.
+       *
+       * Unfortunately, when a reopen is allowed and the IMAP_EXPUNGE_PENDING
+       * flag becomes set (e.g. a flag update to a modified header),
+       * this function will be called by imap_cmd_finish().
+       *
+       * The mx_update_tables() will free and remove these "inactive" headers,
+       * despite that an EXPUNGE was not received for them.
+       * This would result in memory leaks and segfaults due to dangling
+       * pointers in the msn_index and uid_hash.
+       *
+       * So this is another hack to work around the hacks.  We don't want to
+       * remove the messages, so make sure active is on.
+       */
+      h->active = true;
+    }
   }
 
 #ifdef USE_HCACHE