]> granicus.if.org Git - neomutt/commitdiff
Fix use of uninitialized memory in mutt_parse_unmailboxes()
authorThomas Jarosch <thomas.jarosch@intra2net.com>
Sat, 29 Dec 2018 17:44:07 +0000 (18:44 +0100)
committerRichard Russon <rich@flatcap.org>
Tue, 1 Jan 2019 18:23:51 +0000 (18:23 +0000)
The buffer 'tmp' is not initialized when clearing all mailboxes,
therefore don't try to use it in mutt_str_strcasecmp().

The issue was uncovered when switching between different accounts,
"unvirtual-mailboxes *" sometimes cleared regular mailboxes, too.

Important bits from the config:

-----------------------------------
unmailboxes *
mailboxes "+INBOX"

unvirtual-mailboxes *
virtual-mailboxes "virtual INBOX" "notmuch://?query=folder:INBOX%20not%20tag:spam%20not%20tag:archive"
-----------------------------------

-> normal "INBOX" sometimes gets cleared.

valgrind backtrace of the issue without the patch:
==223072== Use of uninitialised value of size 8
==223072==    at 0x593E823: tolower (in /usr/lib64/libc-2.27.so)
==223072==    by 0x4C32BC5: strcasecmp (vg_replace_strmem.c:693)
==223072==    by 0x43FFEB: mutt_parse_unmailboxes (mailbox.c:500)
==223072==    by 0x43B134: mutt_parse_rc_line.part.18 (init.c:3145)
==223072==    by 0x43B42B: mutt_parse_rc_line (init.c:3120)
==223072==    by 0x43B42B: source_rc (init.c:823)
==223072==    by 0x43B93A: parse_source (init.c:1845)
==223072==    by 0x43B134: mutt_parse_rc_line.part.18 (init.c:3145)
==223072==    by 0x43B42B: mutt_parse_rc_line (init.c:3120)
==223072==    by 0x43B42B: source_rc (init.c:823)
==223072==    by 0x43C1B2: mutt_init (init.c:3045)
==223072==    by 0x406EB0: main (main.c:688)

The same problem is also present in the official neomutt 2018-07-16,
I've forward-ported and tested the fix for git HEAD.

The code was slightly refactored to be more readable.

mailbox.c

index 564dab429c417f6ab750d26f9906f104f3e30843..0b8855e35d975b5e961d50153043d19bb37487da 100644 (file)
--- a/mailbox.c
+++ b/mailbox.c
@@ -472,6 +472,7 @@ int mutt_parse_unmailboxes(struct Buffer *buf, struct Buffer *s,
                            unsigned long data, struct Buffer *err)
 {
   char tmp[PATH_MAX];
+  bool tmp_valid = false;
   bool clear_all = false;
 
   while (!clear_all && MoreArgs(s))
@@ -481,11 +482,13 @@ int mutt_parse_unmailboxes(struct Buffer *buf, struct Buffer *s,
     if (mutt_str_strcmp(buf->data, "*") == 0)
     {
       clear_all = true;
+      tmp_valid = false;
     }
     else
     {
       mutt_str_strfcpy(tmp, buf->data, sizeof(tmp));
       mutt_expand_path(tmp, sizeof(tmp));
+      tmp_valid = true;
     }
 
     struct MailboxNode *np = NULL;
@@ -497,8 +500,14 @@ int mutt_parse_unmailboxes(struct Buffer *buf, struct Buffer *s,
       bool norm = ((np->m->magic != MUTT_NOTMUCH) && !(data & MUTT_VIRTUAL));
       bool clear_this = clear_all && (virt || norm);
 
-      if (clear_this || (mutt_str_strcasecmp(tmp, np->m->path) == 0) ||
-          (mutt_str_strcasecmp(tmp, np->m->desc) == 0))
+      /* Compare against path or desc? Ensure 'tmp' is valid */
+      if (!clear_this && tmp_valid)
+      {
+        clear_this = (mutt_str_strcasecmp(tmp, np->m->path) == 0) ||
+                     (mutt_str_strcasecmp(tmp, np->m->desc) == 0);
+      }
+
+      if (clear_this)
       {
 #ifdef USE_SIDEBAR
         mutt_sb_notify_mailbox(np->m, false);