Vulnerability: Mutt before 2.0.2 and NeoMutt before 2020-11-20 did not ensure that $ssl_force_tls was processed if an IMAP server's initial server response was invalid. The connection was not properly closed, and the code could continue attempting to authenticate. This could result in authentication credentials being exposed on an unencrypted connection, or to a machine-in-the-middle.
Vulnerability ID: CVE-2020-28896
CWE: CWE-287
Severity score: 2.9
Project: https://github.com/neomutt/neomutt
Published: 2020-11-23
File: imap/imap.c
Commit message: imap: close connection on all failures

Thanks to Gabriel Salles-Loustau for spotting the problem.

Co-authored-by: Kevin McCarthy <kevin@8t8.us>
Commit author: Richard Russon

Vulnerable version (before fix):
 int imap_open_connection(struct ImapAccountData *adata)       else if ((ans = query_quadoption(C_SslStarttls,                                        _("Secure connection with TLS?"))) == MUTT_ABORT)       {         goto err_close_conn;       }       if (ans == MUTT_YES)       { int imap_open_connection(struct ImapAccountData *adata)           if (mutt_ssl_starttls(adata->conn))           {             mutt_error(_("Could not negotiate TLS connection"));             goto err_close_conn;           }           else           { int imap_open_connection(struct ImapAccountData *adata)     if (C_SslForceTls && (adata->conn->ssf == 0))     {       mutt_error(_("Encrypted connection unavailable"));       goto err_close_conn;     } #endif   } int imap_open_connection(struct ImapAccountData *adata)     if ((adata->conn->ssf == 0) && C_SslForceTls)     {       mutt_error(_("Encrypted connection unavailable"));       goto err_close_conn;     } #endif  int imap_open_connection(struct ImapAccountData *adata)    return 0;  #ifdef USE_SSL err_close_conn:   imap_close_connection(adata); #endif bail:   FREE(&adata->capstr);   return -1; }

Fixed version (after fix):
 int imap_open_connection(struct ImapAccountData *adata)       else if ((ans = query_quadoption(C_SslStarttls,                                        _("Secure connection with TLS?"))) == MUTT_ABORT)       {         goto bail;       }       if (ans == MUTT_YES)       { int imap_open_connection(struct ImapAccountData *adata)           if (mutt_ssl_starttls(adata->conn))           {             mutt_error(_("Could not negotiate TLS connection"));             goto bail;           }           else           { int imap_open_connection(struct ImapAccountData *adata)     if (C_SslForceTls && (adata->conn->ssf == 0))     {       mutt_error(_("Encrypted connection unavailable"));       goto bail;     } #endif   } int imap_open_connection(struct ImapAccountData *adata)     if ((adata->conn->ssf == 0) && C_SslForceTls)     {       mutt_error(_("Encrypted connection unavailable"));       goto bail;     } #endif  int imap_open_connection(struct ImapAccountData *adata)    return 0;  bail:   imap_close_connection(adata);   FREE(&adata->capstr);   return -1; }

Patch:
patch: @@ -761,7 +761,7 @@ int imap_open_connection(struct ImapAccountData *adata)
       else if ((ans = query_quadoption(C_SslStarttls,
                                        _("Secure connection with TLS?"))) == MUTT_ABORT)
       {
-        goto err_close_conn;
+        goto bail;
       }
       if (ans == MUTT_YES)
       {
@@ -776,7 +776,7 @@ int imap_open_connection(struct ImapAccountData *adata)
           if (mutt_ssl_starttls(adata->conn))
           {
             mutt_error(_("Could not negotiate TLS connection"));
-            goto err_close_conn;
+            goto bail;
           }
           else
           {
@@ -791,7 +791,7 @@ int imap_open_connection(struct ImapAccountData *adata)
     if (C_SslForceTls && (adata->conn->ssf == 0))
     {
       mutt_error(_("Encrypted connection unavailable"));
-      goto err_close_conn;
+      goto bail;
     }
 #endif
   }
@@ -807,7 +807,7 @@ int imap_open_connection(struct ImapAccountData *adata)
     if ((adata->conn->ssf == 0) && C_SslForceTls)
     {
       mutt_error(_("Encrypted connection unavailable"));
-      goto err_close_conn;
+      goto bail;
     }
 #endif
 
@@ -824,11 +824,8 @@ int imap_open_connection(struct ImapAccountData *adata)
 
   return 0;
 
-#ifdef USE_SSL
-err_close_conn:
-  imap_close_connection(adata);
-#endif
 bail:
+  imap_close_connection(adata);
   FREE(&adata->capstr);
   return -1;
 }
path: imap/imap.c