Commit c547433c authored by Kevin J. McCarthy's avatar Kevin J. McCarthy

Fix STARTTLS response injection attack.

Thanks again to Damian Poddebniak and Fabian Ising from the Münster
University of Applied Sciences for reporting this issue.  Their
summary in ticket 248 states the issue clearly:

  We found another STARTTLS-related issue in Mutt. Unfortunately, it
  affects SMTP, POP3 and IMAP.

  When the server responds with its "let's do TLS now message", e.g. A
  OK begin TLS\r\n in IMAP or +OK begin TLS\r\n in POP3, Mutt will
  also read any data after the \r\n and save it into some internal
  buffer for later processing. This is problematic, because a MITM
  attacker can inject arbitrary responses.

  There is a nice blogpost by Wietse Venema about a "command
  injection" in postfix (http://www.postfix.org/CVE-2011-0411.html).
  What we have here is the problem in reverse, i.e. not a command
  injection, but a "response injection."

This commit fixes the issue by clearing the CONNECTION input buffer in
mutt_ssl_starttls().

To make backporting this fix easier, the new functions only clear the
top-level CONNECTION buffer; they don't handle nested buffering in
mutt_zstrm.c or mutt_sasl.c.  However both of those wrap the
connection *after* STARTTLS, so this is currently okay.  mutt_tunnel.c
occurs before connecting, but it does not perform any nesting.
parent 34e3a1a3
......@@ -129,6 +129,36 @@ int mutt_socket_write_d (CONNECTION *conn, const char *buf, int len, int dbg)
return sent;
}
/* Checks if the CONNECTION input buffer has unread data.
*
* NOTE: for general use, the function needs to expand to poll nested
* connections. It currently does not to make backporting a security
* fix easier.
*
* STARTTLS occurs before SASL and COMPRESS=DEFLATE processing, and
* mutt_tunnel() does not wrap the connection. So this and the next
* function are safe for current usage in mutt_ssl_starttls().
*/
int mutt_socket_has_buffered_input (CONNECTION *conn)
{
return conn->bufpos < conn->available;
}
/* Clears buffered input from a connection.
*
* NOTE: for general use, the function needs to expand to call nested
* connections. It currently does not to make backporting a security
* fix easier.
*
* STARTTLS occurs before SASL and COMPRESS=DEFLATE processing, and
* mutt_tunnel() does not wrap the connection. So this and the previous
* function are safe for current usage in mutt_ssl_starttls().
*/
void mutt_socket_clear_buffered_input (CONNECTION *conn)
{
conn->bufpos = conn->available = 0;
}
/* poll whether reads would block.
* Returns: >0 if there is data to read,
* 0 if a read would block,
......
......@@ -53,6 +53,8 @@ typedef struct _connection
int mutt_socket_open (CONNECTION* conn);
int mutt_socket_close (CONNECTION* conn);
int mutt_socket_has_buffered_input (CONNECTION *conn);
void mutt_socket_clear_buffered_input (CONNECTION *conn);
int mutt_socket_poll (CONNECTION* conn, time_t wait_secs);
int mutt_socket_readchar (CONNECTION *conn, char *c);
#define mutt_socket_readln(A,B,C) mutt_socket_readln_d(A,B,C,MUTT_SOCK_LOG_CMD)
......
......@@ -199,6 +199,18 @@ int mutt_ssl_starttls (CONNECTION* conn)
int maxbits;
long ssl_options = 0;
if (mutt_socket_has_buffered_input (conn))
{
/* L10N:
The server is not supposed to send data immediately after
confirming STARTTLS. This warns the user that something
weird is going on.
*/
mutt_error _("Warning: clearing unexpected buffered data before STARTTLS");
mutt_sleep (0);
mutt_socket_clear_buffered_input (conn);
}
if (ssl_init())
goto bail;
......
......@@ -218,6 +218,18 @@ static int tls_socket_open (CONNECTION* conn)
int mutt_ssl_starttls (CONNECTION* conn)
{
if (mutt_socket_has_buffered_input (conn))
{
/* L10N:
The server is not supposed to send data immediately after
confirming STARTTLS. This warns the user that something
weird is going on.
*/
mutt_error _("Warning: clearing unexpected buffered data before STARTTLS");
mutt_sleep (0);
mutt_socket_clear_buffered_input (conn);
}
if (tls_init() < 0)
return -1;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment