From: Kevin McCarthy <kevin@8t8.us>
Date: Sat, 21 Nov 2015 23:28:57 +0000 (-0800)
Subject: Remove redundant mbox delimiter check in imap_browse().  (closes #3646)
X-Git-Tag: neomutt-20160404~100
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=c20d0d3babe8d3b161113a39cfe5fab28c1c64f1;p=neomutt

Remove redundant mbox delimiter check in imap_browse().  (closes #3646)

imap_fix_path() removes duplicate and trailing delimiters, so the check
below it was redundant.  This also made it appear list.delim could be
used uninitialized.

Remove the check, but add a check to make sure the "fixed" path has
len>0, to prevent oob accesses of mbox[n-1] below.

Lastly, remove a redundant n=strlen(mbox) inside the initial LIST
processing loop.  The mbox isn't changed from above, so there is no
need to rerun strlen.
---

diff --git a/imap/browse.c b/imap/browse.c
index 6050f2839..e516c4165 100644
--- a/imap/browse.c
+++ b/imap/browse.c
@@ -71,39 +71,44 @@ int imap_browse (char* path, struct browser_state* state)
   /* skip check for parents when at the root */
   if (mx.mbox && mx.mbox[0] != '\0')
   {
-    int rc;
     imap_fix_path (idata, mx.mbox, mbox, sizeof (mbox));
     n = mutt_strlen (mbox);
+  }
+  else
+  {
+    mbox[0] = '\0';
+    n = 0;
+  }
 
+  if (n)
+  {
+    int rc;
     dprint (3, (debugfile, "imap_browse: mbox: %s\n", mbox));
 
     /* if our target exists and has inferiors, enter it if we
      * aren't already going to */
-    if (mbox[n-1] != idata->delim)
+    imap_munge_mbox_name (idata, munged_mbox, sizeof (munged_mbox), mbox);
+    snprintf (buf, sizeof (buf), "%s \"\" %s", list_cmd, munged_mbox);
+    imap_cmd_start (idata, buf);
+    idata->cmdtype = IMAP_CT_LIST;
+    idata->cmddata = &list;
+    do
     {
-      imap_munge_mbox_name (idata, munged_mbox, sizeof (munged_mbox), mbox);
-      snprintf (buf, sizeof (buf), "%s \"\" %s", list_cmd, munged_mbox);
-      imap_cmd_start (idata, buf);
-      idata->cmdtype = IMAP_CT_LIST;
-      idata->cmddata = &list;
-      do
+      list.name = 0;
+      rc = imap_cmd_step (idata);
+      if (rc == IMAP_CMD_CONTINUE && list.name)
       {
-	list.name = 0;
-        rc = imap_cmd_step (idata);
-        if (rc == IMAP_CMD_CONTINUE && list.name)
+        if (!list.noinferiors && list.name[0] &&
+            !imap_mxcmp (list.name, mbox) &&
+            n < sizeof (mbox) - 1)
         {
-          if (!list.noinferiors && list.name[0] &&
-              !imap_mxcmp (list.name, mbox) &&
-            (n = strlen (mbox)) < sizeof (mbox) - 1)
-          {
-            mbox[n++] = list.delim;
-            mbox[n] = '\0';
-          }
+          mbox[n++] = list.delim;
+          mbox[n] = '\0';
         }
       }
-      while (rc == IMAP_CMD_CONTINUE);
-      idata->cmddata = NULL;
     }
+    while (rc == IMAP_CMD_CONTINUE);
+    idata->cmddata = NULL;
 
     /* if we're descending a folder, mark it as current in browser_state */
     if (mbox[n-1] == list.delim)
@@ -161,8 +166,6 @@ int imap_browse (char* path, struct browser_state* state)
       }
     }
   }
-  else
-    mbox[0] = '\0';
 
   /* no namespace, no folder: set folder to host only */
   if (!state->folder)