changeset 30881:a5131a257967

jabber: Fix a pernicious race condition in our cyrus auth code About sasl_getsecret_t, sasl.h reads, in part: outputs: psecret set to password structure which must persist until next call to getsecret **in same connection**, but middleware will erase password data when it's done with it. Clearly this needs to be per-JabberStream*, not a static var. Jan Kaluza noted the static var and then I noted the sasl.h docs. Fixes #11560
author Paul Aurich <paul@darkrain42.org>
date Fri, 27 Aug 2010 04:30:23 +0000
parents dbca01e4c689
children ec12d2883266
files ChangeLog libpurple/protocols/jabber/auth_cyrus.c libpurple/protocols/jabber/jabber.c libpurple/protocols/jabber/jabber.h
diffstat 4 files changed, 18 insertions(+), 11 deletions(-) [+]
line wrap: on
line diff
--- a/ChangeLog	Thu Aug 26 04:37:24 2010 +0000
+++ b/ChangeLog	Fri Aug 27 04:30:23 2010 +0000
@@ -8,16 +8,21 @@
 	* Added ability to use TURN relaying via TCP and TLS (including preference
 	  settings for these).
 
+	Pidgin:
+	* Add support for the Gadu-Gadu protocol in the gevolution plugin to
+	  provide Evolution integration with contacts with GG IDs. (#10709)
+
+	XMPP:
+	* Fix a crash when multiple accounts are simultaneously performing
+	  SASL authentication when built with Cyrus SASL support.  (thanks
+	  to Jan Kaluza) (#11560)
+
 	Yahoo/Yahoo JAPAN:
 	* Stop doing unnecessary lookups of certain alias information.  This
 	  solves deadlocks when a given Yahoo account has a ridiculously large
 	  (>500 buddies) list and may improve login speed for those on slow
 	  connections. (#12532)
 
-	Pidgin:
-	* Add support for the Gadu-Gadu protocol in the gevolution plugin to
-	  provide Evolution integration with contacts with GG IDs. (#10709)
-
 version 2.7.3 (08/10/2010):
 	General:
 	* Use silent build rules for automake >1.11. You can enable verbose
--- a/libpurple/protocols/jabber/auth_cyrus.c	Thu Aug 26 04:37:24 2010 +0000
+++ b/libpurple/protocols/jabber/auth_cyrus.c	Fri Aug 27 04:30:23 2010 +0000
@@ -94,7 +94,6 @@
 	PurpleAccount *account;
 	const char *pw;
 	size_t len;
-	static sasl_secret_t *x = NULL;
 
 	account = purple_connection_get_account(js->gc);
 	pw = purple_account_get_password(account);
@@ -104,15 +103,15 @@
 
 	len = strlen(pw);
 	/* Not an off-by-one because sasl_secret_t defines char data[1] */
-	x = (sasl_secret_t *) realloc(x, sizeof(sasl_secret_t) + len);
-
-	if (!x)
+	/* TODO: This can probably be moved to glib's allocator */
+	js->sasl_secret = malloc(sizeof(sasl_secret_t) + len);
+	if (!js->sasl_secret)
 		return SASL_NOMEM;
 
-	x->len = len;
-	strcpy((char*)x->data, pw);
+	js->sasl_secret->len = len;
+	strcpy((char*)js->sasl_secret->data, pw);
 
-	*secret = x;
+	*secret = js->sasl_secret;
 	return SASL_OK;
 }
 
--- a/libpurple/protocols/jabber/jabber.c	Thu Aug 26 04:37:24 2010 +0000
+++ b/libpurple/protocols/jabber/jabber.c	Fri Aug 27 04:30:23 2010 +0000
@@ -1631,6 +1631,8 @@
 	if(js->sasl_mechs)
 		g_string_free(js->sasl_mechs, TRUE);
 	g_free(js->sasl_cb);
+	/* Note: _not_ g_free.  See auth_cyrus.c:jabber_sasl_cb_secret */
+	free(js->sasl_secret);
 #endif
 	g_free(js->serverFQDN);
 	while(js->commands) {
--- a/libpurple/protocols/jabber/jabber.h	Thu Aug 26 04:37:24 2010 +0000
+++ b/libpurple/protocols/jabber/jabber.h	Fri Aug 27 04:30:23 2010 +0000
@@ -206,6 +206,7 @@
 #ifdef HAVE_CYRUS_SASL
 	sasl_conn_t *sasl;
 	sasl_callback_t *sasl_cb;
+	sasl_secret_t *sasl_secret;
 	const char *current_mech;
 	int auth_fail_count;