CVE-2018-14354: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection') (CWE-78)
value: An issue was discovered in Mutt before 1.10.1 and NeoMutt before 2018-07-16. They allow remote IMAP servers to execute arbitrary commands via backquote characters, related to the mailboxes command associated with a manual subscription or unsubscription.
CWE description: The product constructs all or part of an OS command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended OS command when it is sent to a downstream component.
CVSS v3 base score: 9.8
Published: 2018-07-17T17:29Z
Repository: https://gitlab.com/muttmua/mutt
Language: C
Files changed: imap/command.c, imap/imap.c, imap/imap_private.h, imap/util.c
Commit date: 2018-07-07 19:03:44 -0700
Commit message: Properly quote IMAP mailbox names when (un)subscribing.

When handling automatic subscription (via $imap_check_subscribed), or
manual subscribe/unsubscribe commands, mutt generating a "mailboxes"
command but failed to properly escape backquotes.

Thanks to Jeriko One for the detailed bug report and patch, which this
commit is based upon.

Diff: diff --git a/imap/command.c b/imap/command.c
index c88259815..c79d4f286 100644
--- a/imap/command.c
+++ b/imap/command.c
@@ -840,12 +840,13 @@ static void cmd_parse_lsub (IMAP_DATA* idata, char* s)
 
   dprint (3, (debugfile, "Subscribing to %s\n", list.name));
 
   strfcpy (buf, "mailboxes \"", sizeof (buf));
   mutt_account_tourl (&idata->conn->account, &url);
-  /* escape \ and " */
-  imap_quote_string(errstr, sizeof (errstr), list.name);
+  /* escape \ and ". Also escape ` because the resulting
+   * string will be passed to mutt_parse_rc_line. */
+  imap_quote_string_and_backquotes (errstr, sizeof (errstr), list.name);
   url.path = errstr + 1;
   url.path[strlen(url.path) - 1] = '\0';
   if (!mutt_strcmp (url.user, ImapUser))
     url.user = NULL;
   url_ciss_tostring (&url, buf + 11, sizeof (buf) - 10, 0);
diff --git a/imap/imap.c b/imap/imap.c
index 668203b8e..c3a8ffd08 100644
--- a/imap/imap.c
+++ b/imap/imap.c
@@ -1928,10 +1928,11 @@ int imap_subscribe (char *path, int subscribe)
 {
   IMAP_DATA *idata;
   char buf[LONG_STRING];
   char mbox[LONG_STRING];
   char errstr[STRING];
+  int mblen;
   BUFFER err, token;
   IMAP_MBOX mx;
 
   if (!mx_is_imap (path) || imap_parse_path (path, &mx) || !mx.mbox)
   {
@@ -1949,12 +1950,14 @@ int imap_subscribe (char *path, int subscribe)
   {
     mutt_buffer_init (&token);
     mutt_buffer_init (&err);
     err.data = errstr;
     err.dsize = sizeof (errstr);
-    snprintf (mbox, sizeof (mbox), "%smailboxes \"%s\"",
-              subscribe ? "" : "un", path);
+    mblen = snprintf (mbox, sizeof (mbox), "%smailboxes ",
+                      subscribe ? "" : "un");
+    imap_quote_string_and_backquotes (mbox + mblen, sizeof(mbox) - mblen,
+                                      path);
     if (mutt_parse_rc_line (mbox, &token, &err))
       dprint (1, (debugfile, "Error adding subscribed mailbox: %s\n", errstr));
     FREE (&token.data);
   }
 
diff --git a/imap/imap_private.h b/imap/imap_private.h
index 312fbfe4f..349c5a49d 100644
--- a/imap/imap_private.h
+++ b/imap/imap_private.h
@@ -299,11 +299,12 @@ char* imap_get_qualifier (char* buf);
 int imap_mxcmp (const char* mx1, const char* mx2);
 char* imap_next_word (char* s);
 time_t imap_parse_date (char* s);
 void imap_make_date (char* buf, time_t timestamp);
 void imap_qualify_path (char *dest, size_t len, IMAP_MBOX *mx, char* path);
-void imap_quote_string (char* dest, size_t slen, const char* src);
+void imap_quote_string (char* dest, size_t dlen, const char* src);
+void imap_quote_string_and_backquotes (char *dest, size_t dlen, const char *src);
 void imap_unquote_string (char* s);
 void imap_munge_mbox_name (IMAP_DATA *idata, char *dest, size_t dlen, const char *src);
 void imap_unmunge_mbox_name (IMAP_DATA *idata, char *s);
 int imap_wordcasecmp(const char *a, const char *b);
 
diff --git a/imap/util.c b/imap/util.c
index 914c93c3c..3274a70c5 100644
--- a/imap/util.c
+++ b/imap/util.c
@@ -606,15 +606,13 @@ void imap_qualify_path (char *dest, size_t len, IMAP_MBOX *mx, char* path)
 
   url_ciss_tostring (&url, dest, len, 0);
 }
 
 
-/* imap_quote_string: quote string according to IMAP rules:
- *   surround string with quotes, escape " and \ with \ */
-void imap_quote_string (char *dest, size_t dlen, const char *src)
+static void _imap_quote_string (char *dest, size_t dlen, const char *src,
+                                const char *to_quote)
 {
-  static const char quote[] = "\"\\";
   char *pt;
   const char *s;
 
   pt = dest;
   s  = src;
@@ -623,11 +621,11 @@ void imap_quote_string (char *dest, size_t dlen, const char *src)
   /* save room for trailing quote-char */
   dlen -= 2;
 
   for (; *s && dlen; s++)
   {
-    if (strchr (quote, *s))
+    if (strchr (to_quote, *s))
     {
       dlen -= 2;
       if (!dlen)
 	break;
       *pt++ = '\\';
@@ -641,10 +639,27 @@ void imap_quote_string (char *dest, size_t dlen, const char *src)
   }
   *pt++ = '"';
   *pt = 0;
 }
 
+/* imap_quote_string: quote string according to IMAP rules:
+ *   surround string with quotes, escape " and \ with \ */
+void imap_quote_string (char *dest, size_t dlen, const char *src)
+{
+  _imap_quote_string (dest, dlen, src, "\"\\");
+}
+
+/* imap_quote_string_and_backquotes: quote string according to IMAP rules:
+ *   surround string with quotes, escape " and \ with \.
+ * Additionally, escape backquotes with \ to protect against code injection
+ * when using the resulting string in mutt_parse_rc_line().
+ */
+void imap_quote_string_and_backquotes (char *dest, size_t dlen, const char *src)
+{
+  _imap_quote_string (dest, dlen, src, "\"\\`");
+}
+
 /* imap_unquote_string: equally stupid unquoting routine */
 void imap_unquote_string (char *s)
 {
   char *d = s;
 

Vulnerable code:   /* escape \ and " */
  imap_quote_string(errstr, sizeof (errstr), list.name);
    snprintf (mbox, sizeof (mbox), "%smailboxes \"%s\"",
              subscribe ? "" : "un", path);
void imap_quote_string (char* dest, size_t slen, const char* src);
/* imap_quote_string: quote string according to IMAP rules:
 *   surround string with quotes, escape " and \ with \ */
void imap_quote_string (char *dest, size_t dlen, const char *src)
  static const char quote[] = "\"\\";
    if (strchr (quote, *s))
Fixed code:   /* escape \ and ". Also escape ` because the resulting
   * string will be passed to mutt_parse_rc_line. */
  imap_quote_string_and_backquotes (errstr, sizeof (errstr), list.name);
  int mblen;
    mblen = snprintf (mbox, sizeof (mbox), "%smailboxes ",
                      subscribe ? "" : "un");
    imap_quote_string_and_backquotes (mbox + mblen, sizeof(mbox) - mblen,
                                      path);
void imap_quote_string (char* dest, size_t dlen, const char* src);
void imap_quote_string_and_backquotes (char *dest, size_t dlen, const char *src);
static void _imap_quote_string (char *dest, size_t dlen, const char *src,
                                const char *to_quote)
    if (strchr (to_quote, *s))
/* imap_quote_string: quote string according to IMAP rules:
 *   surround string with quotes, escape " and \ with \ */
void imap_quote_string (char *dest, size_t dlen, const char *src)
{
  _imap_quote_string (dest, dlen, src, "\"\\");
}

/* imap_quote_string_and_backquotes: quote string according to IMAP rules:
 *   surround string with quotes, escape " and \ with \.
 * Additionally, escape backquotes with \ to protect against code injection
 * when using the resulting string in mutt_parse_rc_line().
 */
void imap_quote_string_and_backquotes (char *dest, size_t dlen, const char *src)
{
  _imap_quote_string (dest, dlen, src, "\"\\`");
}