CVE-2018-14357: 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 an automatic subscription.
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://github.com/neomutt/neomutt
Language: C
Files changed: imap/auth_login.c, imap/command.c, imap/imap.c, imap/imap_private.h, imap/util.c
Commit date: 2018-07-06 00:52:55 +0100
Commit message: quote imap strings more carefully

Co-authored-by: JerikoOne <jeriko.one@gmx.us>

Diff: diff --git a/imap/auth_login.c b/imap/auth_login.c
index f6de06612..0f4445b45 100644
--- a/imap/auth_login.c
+++ b/imap/auth_login.c
@@ -63,12 +63,12 @@ enum ImapAuthRes imap_auth_login(struct ImapData *idata, const char *method)
   if (mutt_account_getpass(&idata->conn->account) < 0)
     return IMAP_AUTH_FAILURE;
 
   mutt_message(_("Logging in..."));
 
-  imap_quote_string(q_user, sizeof(q_user), idata->conn->account.user);
-  imap_quote_string(q_pass, sizeof(q_pass), idata->conn->account.pass);
+  imap_quote_string(q_user, sizeof(q_user), idata->conn->account.user, false);
+  imap_quote_string(q_pass, sizeof(q_pass), idata->conn->account.pass, false);
 
   /* don't print the password unless we're at the ungodly debugging level
    * of 5 or higher */
 
   if (DebugLevel < IMAP_LOG_PASS)
diff --git a/imap/command.c b/imap/command.c
index e3378e1e3..859f98727 100644
--- a/imap/command.c
+++ b/imap/command.c
@@ -497,11 +497,11 @@ static void cmd_parse_lsub(struct ImapData *idata, char *s)
   mutt_debug(3, "Subscribing to %s\n", list.name);
 
   mutt_str_strfcpy(buf, "mailboxes \"", sizeof(buf));
   mutt_account_tourl(&idata->conn->account, &url);
   /* escape \ and " */
-  imap_quote_string(errstr, sizeof(errstr), list.name);
+  imap_quote_string(errstr, sizeof(errstr), list.name, true);
   url.path = errstr + 1;
   url.path[strlen(url.path) - 1] = '\0';
   if (mutt_str_strcmp(url.user, ImapUser) == 0)
     url.user = NULL;
   url_tostring(&url, buf + 11, sizeof(buf) - 11, 0);
diff --git a/imap/imap.c b/imap/imap.c
index ea12470b0..40f620a2e 100644
--- a/imap/imap.c
+++ b/imap/imap.c
@@ -462,29 +462,29 @@ static int compile_search(struct Context *ctx, const struct Pattern *pat, struct
         {
           mutt_error(_("Header search without header name: %s"), pat->p.str);
           return -1;
         }
         *delim = '\0';
-        imap_quote_string(term, sizeof(term), pat->p.str);
+        imap_quote_string(term, sizeof(term), pat->p.str, false);
         mutt_buffer_addstr(buf, term);
         mutt_buffer_addch(buf, ' ');
 
         /* and field */
         *delim = ':';
         delim++;
         SKIPWS(delim);
-        imap_quote_string(term, sizeof(term), delim);
+        imap_quote_string(term, sizeof(term), delim, false);
         mutt_buffer_addstr(buf, term);
         break;
       case MUTT_BODY:
         mutt_buffer_addstr(buf, "BODY ");
-        imap_quote_string(term, sizeof(term), pat->p.str);
+        imap_quote_string(term, sizeof(term), pat->p.str, false);
         mutt_buffer_addstr(buf, term);
         break;
       case MUTT_WHOLE_MSG:
         mutt_buffer_addstr(buf, "TEXT ");
-        imap_quote_string(term, sizeof(term), pat->p.str);
+        imap_quote_string(term, sizeof(term), pat->p.str, false);
         mutt_buffer_addstr(buf, term);
         break;
       case MUTT_SERVERSEARCH:
       {
         struct ImapData *idata = ctx->data;
@@ -493,11 +493,11 @@ static int compile_search(struct Context *ctx, const struct Pattern *pat, struct
           mutt_error(_("Server-side custom search not supported: %s"), pat->p.str);
           return -1;
         }
       }
         mutt_buffer_addstr(buf, "X-GM-RAW ");
-        imap_quote_string(term, sizeof(term), pat->p.str);
+        imap_quote_string(term, sizeof(term), pat->p.str, false);
         mutt_buffer_addstr(buf, term);
         break;
     }
   }
 
diff --git a/imap/imap_private.h b/imap/imap_private.h
index 6ae9f05ad..278d4a2b1 100644
--- a/imap/imap_private.h
+++ b/imap/imap_private.h
@@ -326,11 +326,11 @@ void imap_cachepath(struct ImapData *idata, const char *mailbox, char *dest, siz
 int imap_get_literal_count(const char *buf, unsigned int *bytes);
 char *imap_get_qualifier(char *buf);
 int imap_mxcmp(const char *mx1, const char *mx2);
 char *imap_next_word(char *s);
 void imap_qualify_path(char *dest, size_t len, struct ImapMbox *mx, char *path);
-void imap_quote_string(char *dest, size_t dlen, const char *src);
+void imap_quote_string(char *dest, size_t dlen, const char *src, bool quote_backtick);
 void imap_unquote_string(char *s);
 void imap_munge_mbox_name(struct ImapData *idata, char *dest, size_t dlen, const char *src);
 void imap_unmunge_mbox_name(struct ImapData *idata, char *s);
 int imap_account_match(const struct Account *a1, const struct Account *a2);
 void imap_get_parent(char *output, const char *mbox, size_t olen, char delim);
diff --git a/imap/util.c b/imap/util.c
index 23e4f7093..614d87a56 100644
--- a/imap/util.c
+++ b/imap/util.c
@@ -796,13 +796,16 @@ void imap_qualify_path(char *dest, size_t len, struct ImapMbox *mx, char *path)
  * @param dlen Length of the buffer
  * @param src  String to be quoted
  *
  * Surround string with quotes, escape " and \ with backslash
  */
-void imap_quote_string(char *dest, size_t dlen, const char *src)
+void imap_quote_string(char *dest, size_t dlen, const char *src, bool quote_backtick)
 {
-  static const char quote[] = "\"\\";
+  const char *quote = "`\"\\";
+  if (!quote_backtick)
+    quote++;
+
   char *pt = dest;
   const char *s = src;
 
   *pt++ = '"';
   /* save room for trailing quote-char */
@@ -872,11 +875,11 @@ void imap_unquote_string(char *s)
 void imap_munge_mbox_name(struct ImapData *idata, char *dest, size_t dlen, const char *src)
 {
   char *buf = mutt_str_strdup(src);
   imap_utf_encode(idata, &buf);
 
-  imap_quote_string(dest, dlen, buf);
+  imap_quote_string(dest, dlen, buf, false);
 
   FREE(&buf);
 }
 
 /**

Vulnerable code:   imap_quote_string(q_user, sizeof(q_user), idata->conn->account.user);
  imap_quote_string(q_pass, sizeof(q_pass), idata->conn->account.pass);
  imap_quote_string(errstr, sizeof(errstr), list.name);
        imap_quote_string(term, sizeof(term), pat->p.str);
        imap_quote_string(term, sizeof(term), delim);
        imap_quote_string(term, sizeof(term), pat->p.str);
        imap_quote_string(term, sizeof(term), pat->p.str);
        imap_quote_string(term, sizeof(term), pat->p.str);
void imap_quote_string(char *dest, size_t dlen, const char *src);
void imap_quote_string(char *dest, size_t dlen, const char *src)
  static const char quote[] = "\"\\";
  imap_quote_string(dest, dlen, buf);
Fixed code:   imap_quote_string(q_user, sizeof(q_user), idata->conn->account.user, false);
  imap_quote_string(q_pass, sizeof(q_pass), idata->conn->account.pass, false);
  imap_quote_string(errstr, sizeof(errstr), list.name, true);
        imap_quote_string(term, sizeof(term), pat->p.str, false);
        imap_quote_string(term, sizeof(term), delim, false);
        imap_quote_string(term, sizeof(term), pat->p.str, false);
        imap_quote_string(term, sizeof(term), pat->p.str, false);
        imap_quote_string(term, sizeof(term), pat->p.str, false);
void imap_quote_string(char *dest, size_t dlen, const char *src, bool quote_backtick);
void imap_quote_string(char *dest, size_t dlen, const char *src, bool quote_backtick)
  const char *quote = "`\"\\";
  if (!quote_backtick)
    quote++;

  imap_quote_string(dest, dlen, buf, false);