security review and patches for libpurple

[Paul Aurich]

And Ethan Blanton spoke on 08/11/2011 08:19 AM, saying:
> * auth_cyrus.c
> 
>   This is bogus like the first patch; len is strlen(pw).  If we can
>   determine the size of sasl_secret_t.data, we need to use that.  I
>   haven't looked into this yet.

sasl_secret_t.data is malloc()d directly prior in the function, so it can
never be an overflow -- the object is defined the same as the rejected
patch for stringref.c.

From sasl.h:
typedef struct sasl_secret {
    unsigned long len;
    unsigned char data[1];		/* variable sized */
} sasl_secret_t;

From auth_cyrus.c:
static int jabber_sasl_cb_secret(sasl_conn_t *conn, void *ctx, int id,
sasl_secret_t **secret)
{
...
	len = strlen(pw);
	/* Not an off-by-one because sasl_secret_t defines char data[1] */
	/* 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;

	js->sasl_secret->len = len;
	strcpy((char*)js->sasl_secret->data, pw);

	*secret = js->sasl_secret;
	return SASL_OK;
}

(It does seem like there might now be a memory leak in here if Cyrus calls
the secret_cb function multiple times [prior to fixing a race condition we
had, it was a realloc of a static struct], but that's unrelated).

~Paul

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 900 bytes
Desc: OpenPGP digital signature
URL: <http://pidgin.im/cgi-bin/mailman/private/security/attachments/20110811/d9aa790a/attachment.pgp>