Merge pull request #3312 from hamarituc/cyrus-imapd-2.4

Fix two use after free segfaults

Author: elliefm

Diff for: imap/backend.c

@@ -284,7 +284,6 @@ static int backend_authenticate(struct backend *s, struct protocol_t *prot,
     struct sockaddr_storage saddr_l, saddr_r;
     char remoteip[60], localip[60];
     socklen_t addrsize;
-    int local_cb = 0;
     char buf[2048], optstr[128], *p;
     const char *mech_conf, *pass;
 
@@ -301,8 +300,8 @@ static int backend_authenticate(struct backend *s, struct protocol_t *prot,
     if(iptostring((struct sockaddr *)&saddr_l, addrsize, localip, 60) != 0)
 	return SASL_FAIL;
 
+    s->sasl_cb = NULL;
     if (!cb) {
-	local_cb = 1;
 	strlcpy(optstr, s->hostname, sizeof(optstr));
 	p = strchr(optstr, '.');
 	if (p) *p = '\0';
@@ -313,6 +312,7 @@ static int backend_authenticate(struct backend *s, struct protocol_t *prot,
 			      config_getstring(IMAPOPT_PROXY_AUTHNAME),
 			      config_getstring(IMAPOPT_PROXY_REALM),
 			      pass);
+        s->sasl_cb = cb;
     }
 
     /* Require proxying if we have an "interesting" userid (authzid) */
@@ -321,14 +321,16 @@ static int backend_authenticate(struct backend *s, struct protocol_t *prot,
 			(prot->sasl_cmd.parse_success ? SASL_SUCCESS_DATA : 0),
 			&s->saslconn);
     if (r != SASL_OK) {
-	if (local_cb) free_callbacks(cb);
-	return r;
+        free_callbacks(s->sasl_cb);
+        s->sasl_cb = NULL;
+        return r;
     }
 
     r = sasl_setprop(s->saslconn, SASL_SEC_PROPS, &secprops);
     if (r != SASL_OK) {
-	if (local_cb) free_callbacks(cb);
-	return r;
+        free_callbacks(s->sasl_cb);
+        s->sasl_cb = NULL;
+        return r;
     }
 
     /* Get SASL mechanism list.  We can force a particular
@@ -377,9 +379,6 @@ static int backend_authenticate(struct backend *s, struct protocol_t *prot,
 					 &s->capability, NULL,
 					 prot->tls_cmd.auto_capa)));
 
-    /* xxx unclear that this is correct */
-    if (local_cb) free_callbacks(cb);
-
     if (r == SASL_OK) {
 	prot_setsasl(s->in, s->saslconn);
 	prot_setsasl(s->out, s->saslconn);
@@ -722,6 +721,12 @@ void backend_disconnect(struct backend *s)
 	s->saslconn = NULL;
     }
 
+    /* Free any SASL callbacks */
+    if (s->sasl_cb) {
+	free_callbacks(s->sasl_cb);
+	s->sasl_cb = NULL;
+    }
+
     /* free last_result buffer */
     buf_free(&s->last_result);
 }

Diff for: imap/backend.h

@@ -71,6 +71,7 @@ struct backend {
     struct prot_waitevent *timeout; /* event for idle timeout */
 
     sasl_conn_t *saslconn;
+    sasl_callback_t *sasl_cb;
 #ifdef HAVE_SSL
     SSL *tlsconn;
     SSL_SESSION *tlssess;

Diff for: imap/mupdate-client.c

@@ -105,7 +105,6 @@ int mupdate_connect(const char *server,
 		    sasl_callback_t *cbs)
 {
     mupdate_handle *h = NULL;
-    int local_cbs = 0;
     const char *status = NULL;
 
     if(!handle)
@@ -122,21 +121,21 @@ int mupdate_connect(const char *server,
     h = xzmalloc(sizeof(mupdate_handle));
     *handle = h;
 
+    h->sasl_cb = NULL;
     if(!cbs) {
-	local_cbs = 1;
 	cbs = mysasl_callbacks(config_getstring(IMAPOPT_MUPDATE_USERNAME),
 			       config_getstring(IMAPOPT_MUPDATE_AUTHNAME),
 			       config_getstring(IMAPOPT_MUPDATE_REALM),
 			       config_getstring(IMAPOPT_MUPDATE_PASSWORD));
+        h->sasl_cb = cbs;
     }
 
     h->conn = backend_connect(NULL, server, &mupdate_protocol,
 			      "", cbs, &status);
 
-    /* xxx unclear that this is correct, but it prevents a memory leak */
-    if (local_cbs) free_callbacks(cbs);
-
     if (!h->conn) {
+        free_callbacks(h->sasl_cb);
+        h->sasl_cb = NULL;
         syslog(LOG_ERR, "mupdate_connect failed: %s", status ? status : "no auth status");
 	return MUPDATE_NOCONN;
     }
@@ -157,6 +156,9 @@ void mupdate_disconnect(mupdate_handle **hp)
     backend_disconnect(h->conn);
     free(h->conn);
 
+    free_callbacks(h->sasl_cb);
+    h->sasl_cb = NULL;
+
     buf_free(&(h->tag));
     buf_free(&(h->cmd));
     buf_free(&(h->arg1));

Diff for: imap/mupdate.h

@@ -62,6 +62,7 @@
 #include "global.h"
 
 struct mupdate_handle_s {
+    sasl_callback_t *sasl_cb;
     struct backend *conn;
 
     /* For keeping track of what tag # is next */