Olaf Hering wrote: > On Mon, Jan 06, Olaf Hering wrote: > > > Current HEAD crashes if IMAP login fails. The backtrace is: > > > > (gdb) bt > > #0 __strstr_sse2 (haystack_start=0x0, needle_start=0x4c0276 "AUTH=LOGIN") > > at ../string/strstr.c:63 > > #1 0x00000000004a76ac in imap_auth_sasl (idata=0x8b6a90, method=0x8daf90 > > "login") at auth_sasl.c:73 > > #2 0x00000000004a7355 in imap_authenticate (idata=0x8b6a90) at auth.c:79 > > #3 0x00000000004a038c in imap_conn_find (account=0x7fffffffb910, > > flags=<optimized out>) at imap.c:373 > > #4 0x00000000004a05e5 in imap_get_mailbox (path=<optimized out>, > > hidata=0x7fffffffbad8, buf=0x7fffffffc2e0 "\360\302\377\377\377\177", > > blen=1024) at imap.c:1443 > > #5 0x00000000004a1920 in imap_buffy_check (force=<optimized out>) at > > imap.c:1485 > > #6 0x0000000000410208 in mutt_buffy_check (force=1) at buffy.c:403 > > #7 0x000000000041f76a in mutt_index_menu () at curs_main.c:522 > > #8 0x000000000043b111 in main (argc=1, argv=<optimized out>) at main.c:1053 > > > > idata->capstr remains unset unless cmd_parse_capability is called. How > > should a failed login be handled to avoid the crash? > > Any word on that?
Hi Olaf, Would you mind applying this patch and running mutt with -d 5 to get debugging output, just so I have a clearer picture where imap_open_connection is bailing. I have added a NONULL around the capstr, so hopefully it won't segfault. Brendan, it looks like a "bail" in imap_open_connection() leaves the state as IMAP_CONNECTED but frees the capstr. So imap_conn_find() happily goes right on trying authenticators. I'm guessing that's okay for some authenticators, and that the strstr in imap_auth_sasl() just needs a NONULL. Would you mind confirming that? -- Kevin J. McCarthy GPG Fingerprint: 8975 A9B3 3AA3 7910 385C 5308 ADEF 7684 8031 6BDA http://www.8t8.us/configs/gpg-key-transition-statement.txt
# HG changeset patch # User Kevin McCarthy <[email protected]> # Date 1434151644 25200 # Fri Jun 12 16:27:24 2015 -0700 # Node ID a752e93de62460539d6bf97e67267ed5b75e1b4d # Parent 17a4f92e4a95d0225ce120b72597e8b72294a8ad [mq]: imap-debug diff --git a/imap/auth_sasl.c b/imap/auth_sasl.c --- a/imap/auth_sasl.c +++ b/imap/auth_sasl.c @@ -65,17 +65,17 @@ return IMAP_AUTH_FAILURE; if (mutt_bit_isset (idata->capabilities, AUTH_ANON) && (!idata->conn->account.user[0] || !ascii_strncmp (idata->conn->account.user, "anonymous", 9))) rc = sasl_client_start (saslconn, "AUTH=ANONYMOUS", NULL, &pc, &olen, &mech); } else if (!ascii_strcasecmp ("login", method) && - !strstr (idata->capstr, "AUTH=LOGIN")) + !strstr (NONULL (idata->capstr), "AUTH=LOGIN")) /* do not use SASL login for regular IMAP login (#3556) */ return IMAP_AUTH_UNAVAIL; if (rc != SASL_OK && rc != SASL_CONTINUE) do { rc = sasl_client_start (saslconn, method, &interaction, &pc, &olen, &mech); diff --git a/imap/imap.c b/imap/imap.c --- a/imap/imap.c +++ b/imap/imap.c @@ -409,49 +409,60 @@ idata->state = IMAP_CONNECTED; if (imap_cmd_step (idata) != IMAP_CMD_CONTINUE) { imap_close_connection (idata); return -1; } + dprint (5, (debugfile, "imap_open_connection: buf is %s\n", idata->buf)); + if (ascii_strncasecmp ("* OK", idata->buf, 4) == 0) { if (ascii_strncasecmp ("* OK [CAPABILITY", idata->buf, 16) && imap_check_capabilities (idata)) + { + dprint (5, (debugfile, "imap_open_connection: capability check failed: bailing.\n")); goto bail; + } #if defined(USE_SSL) /* Attempt STARTTLS if available and desired. */ if (!idata->conn->ssf && (option(OPTSSLFORCETLS) || mutt_bit_isset (idata->capabilities, STARTTLS))) { int rc; if (option(OPTSSLFORCETLS)) rc = M_YES; else if ((rc = query_quadoption (OPT_SSLSTARTTLS, _("Secure connection with TLS?"))) == -1) goto err_close_conn; if (rc == M_YES) { if ((rc = imap_exec (idata, "STARTTLS", IMAP_CMD_FAIL_OK)) == -1) + { + dprint (5, (debugfile, "imap_open_connection: STARTTLS failed: bailing.\n")); goto bail; + } if (rc != -2) { if (mutt_ssl_starttls (idata->conn)) { mutt_error (_("Could not negotiate TLS connection")); mutt_sleep (1); goto err_close_conn; } else { /* RFC 2595 demands we recheck CAPABILITY after TLS completes. */ if (imap_exec (idata, "CAPABILITY", 0)) + { + dprint (5, (debugfile, "imap_open_connection: post-tls capability check failed: bailing.\n")); goto bail; + } } } } } if (option(OPTSSLFORCETLS) && ! idata->conn->ssf) { mutt_error _("Encrypted connection unavailable"); @@ -459,32 +470,37 @@ goto err_close_conn; } #endif } else if (ascii_strncasecmp ("* PREAUTH", idata->buf, 9) == 0) { idata->state = IMAP_AUTHENTICATED; if (imap_check_capabilities (idata) != 0) + { + dprint (5, (debugfile, "imap_open_connection: PREAUTH capability check failed: bailing.\n")); goto bail; + } FREE (&idata->capstr); } else { + dprint (5, (debugfile, "imap_open_connection: bailing on unexpected server response.\n")); imap_error ("imap_open_connection()", buf); goto bail; } return 0; #if defined(USE_SSL) err_close_conn: imap_close_connection (idata); #endif bail: + dprint (5, (debugfile, "imap_open_connection: bailing. freeing capstr\n")); FREE (&idata->capstr); return -1; } void imap_close_connection(IMAP_DATA* idata) { if (idata->state != IMAP_DISCONNECTED) {
signature.asc
Description: PGP signature
