Oswald Buddenhagen wrote:
> On Wed, Jun 17, 2015 at 07:58:23PM -0700, Kevin J. McCarthy wrote:
> > Ah!  I misunderstood the cause of the crash.  If your first login fails,
> > then the *second* time you try to login, you were getting the crash.  Is
> > that right?
> > 
> > I see why this is happening: it's freeing the capstr, even if
> > authentication fails.
> > 
> > So the question is whether the capstr should be kept,
> > 
> yes. until you disconnect or authentication succeeds.
> 
> > or whether, the code should do something like:
> > 
> no.

I'm attaching a patch that does this.  It keeps the NONULL check in
imap_auth_sasl, because it still seems possible this could be called
without capstr being set.  It also changes imap_open_connection() to
free the capstr only after a successful authentication.

-- 
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 1434658369 25200
#      Thu Jun 18 13:12:49 2015 -0700
# Node ID 70677fa1cd3b682fbfd13cb4abc608c4a5491aab
# Parent  17a4f92e4a95d0225ce120b72597e8b72294a8ad
Fix IMAP segfault due to NULL capstr.

After a failed login, the connection is left open but capstr is freed.
If a second login attempt is made, imap_auth_sasl was trying to strstr
using the NULL capstr.

Add a NONULL around the capstr parameter to strstr.  Change
imap_conn_find() to keep the capstr around until a successful
authentication occurs.

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
@@ -368,25 +368,24 @@
 
   if (idata->state == IMAP_DISCONNECTED)
     imap_open_connection (idata);
   if (idata->state == IMAP_CONNECTED)
   {
     if (!imap_authenticate (idata))
     {
       idata->state = IMAP_AUTHENTICATED;
+      FREE (&idata->capstr);
       new = 1;
       if (idata->conn->ssf)
        dprint (2, (debugfile, "Communication encrypted at %d bits\n",
                    idata->conn->ssf));
     }
     else
       mutt_account_unsetpass (&idata->conn->account);
-
-    FREE (&idata->capstr);
   }
   if (new && idata->state == IMAP_AUTHENTICATED)
   {
     /* capabilities may have changed */
     imap_exec (idata, "CAPABILITY", IMAP_CMD_QUEUE);
     /* get root delimiter, '/' as default */
     idata->delim = '/';
     imap_exec (idata, "LIST \"\" \"\"", IMAP_CMD_QUEUE);

Attachment: signature.asc
Description: PGP signature

Reply via email to