xorl %eax, %eax

CVE-2010-3711: Pidgin Multiple Remote NULL Pointer Dereferences

with 3 comments

Recently, Daniel Atallah discovered and reported a flaw in the usage of purple_base64_decode() routine according to the project’s security advisory. These vulnerabilities affect the popular instant messaging software prior to 2.7.4 version. First of all, let’s a have a look at the purple_base64_decode() function that resides at libpurple/util.c file.

guchar *
purple_base64_decode(const char *str, gsize *ret_len)
{
        /*
         * We want to allow ret_len to be NULL for backward compatibility,
         * but g_base64_decode() requires a valid length variable.  So if
         * ret_len is NULL then pass in a dummy variable.
         */
        gsize unused;
        return g_base64_decode(str, ret_len != NULL ? ret_len : &unused);
}

It’s nothing more than a wrapper around g_base64_decode() and it retuns g_base64_decode()’s return value which as we can read is:

Returns: a newly allocated buffer containing the binary 
         data that text represents. The returned buffer 
         must be freed with g_free(). 
         

So, here is nothing to worry about according to the documentation. However, a closer look at GLib’s g_base64_decode() implementation which can be found at glib/gbase64.c reveals something different.

/**
 * g_base64_decode:
 * @text: zero-terminated string with base64 text to decode
 * @out_len: The length of the decoded data is written here
 *
 * Decode a sequence of Base-64 encoded text into binary data
 *
 * Return value: a newly allocated buffer containing the binary data
 *               that @text represents. The returned buffer must
 *               be freed with g_free().
 *
 * Since: 2.12
 */
guchar *
g_base64_decode (const gchar *text,
                 gsize       *out_len)
{
  guchar *ret;
  gsize input_length;
  gint state = 0;
  guint save = 0;

  g_return_val_if_fail (text != NULL, NULL);
  g_return_val_if_fail (out_len != NULL, NULL);

  input_length = strlen (text);

  /* We can use a smaller limit here, since we know the saved state is 0,
     +1 used to avoid calling g_malloc0(0), and hence retruning NULL */
  ret = g_malloc0 ((input_length / 4) * 3 + 1);

  *out_len = g_base64_decode_step (text, input_length, ret, &state, &save);

  return ret;
}

The above code is part of 2.26.1 release of GLib but the important thing to notice here is that g_base64_decode() could return NULL if g_malloc0() fails. This means that using purple_base64_decode() in Pidgin’s code without checking for NULL return values can return in NULL pointer dereferences when malformed Base64 encoded data are received.
This happens in eight situations that you can see below…

NTLM Parsing
This code is available at libpurple/ntlm.c and here is the susceptible call:

guint8 *
purple_ntlm_parse_type2(const gchar *type2, guint32 *flags)
{
        gsize retlen;
        struct type2_message *tmsg;
        static guint8 nonce[8];

        tmsg = (struct type2_message*)purple_base64_decode(type2, &retlen);
        memcpy(nonce, tmsg->nonce, 8);
        if (flags != NULL)
                *flags = GUINT16_FROM_LE(tmsg->flags);
        g_free(tmsg);

        return nonce;
}

The memcpy(3)’s attempt to read from ‘tmsg->nonce’ will result in a NULL pointer dereference if purple_base64_decode() returned NULL. The fix for this routine was to patch it like this:

 	tmsg = (struct type2_message*)purple_base64_decode(type2, &retlen);
-	memcpy(nonce, tmsg->nonce, 8);
-	if (flags != NULL)
-		*flags = GUINT16_FROM_LE(tmsg->flags);
+	if (tmsg != NULL && retlen >= (sizeof(struct type2_message) - 1)) {
+		memcpy(nonce, tmsg->nonce, 8);
+		if (flags != NULL)
+			*flags = GUINT16_FROM_LE(tmsg->flags);
+	} else {
+		purple_debug_error("ntlm", "Unable to parse type2 message - returning empty nonce.\n");
+		memset(nonce, 0, 8);
+	}
 	g_free(tmsg);

That checks for both the returned value and the size of the decoded message before calling memcpy(3) to copy it.

MSN SLP Session Request
Now, we move to libpurple/protocols/msn/slp.c to find the following source code…

static void
got_sessionreq(MsnSlpCall *slpcall, const char *branch,
                           const char *euf_guid, const char *context)
{
        gboolean accepted = FALSE;

        if (!strcmp(euf_guid, MSN_OBJ_GUID))
        {
   ...
        else if (!strcmp(euf_guid, MSN_FT_GUID))
        {
                /* File Transfer */
   ...
                header = (MsnFileContext *)purple_base64_decode(context, &bin_len);
                if (bin_len >= sizeof(MsnFileContext) - 1 &&
                        (header->version == 2 ||
                         (header->version == 3 && header->length == sizeof(MsnFileContext) + 63))) {
                        file_size = GUINT64_FROM_LE(header->file_size);

                        file_name = g_convert((const gchar *)&header->file_name,
                                              MAX_FILE_NAME_LEN * 2,
                                              "UTF-8", "UTF-16LE",
                                              NULL, NULL, NULL);
   ...
}

Here, file transfer request of the MSN protocol use Base64 messages. Since there is no check of the return value, this can lead to NULL pointer dereference. Obviously, the patch was:

                header = (MsnFileContext *)purple_base64_decode(context, &bin_len);
-               if (bin_len >= sizeof(MsnFileContext) - 1 &&
+               if (header != NULL && bin_len >= sizeof(MsnFileContext) - 1 &&
                        (header->version == 2 ||
                         (header->version == 3 && header->length == sizeof(MsnFileContext) + 63))) {
                        file_size = GUINT64_FROM_LE(header->file_size);

That adds the length and the NULL missing checks.

MySpace IM Get Binary
The next vulnerability can be found at libpurple/protocols/myspace/message.c where the following routine is located.

static gboolean
msim_msg_get_binary_from_element(MsimMessageElement *elem, gchar **binary_data, gsize *binary_length)
{
        GString *gs;

        g_return_val_if_fail(elem != NULL, FALSE);

        switch (elem->type) {
                case MSIM_TYPE_RAW:
                         /* Incoming messages are tagged with MSIM_TYPE_RAW, and
                         * converted appropriately. They can still be "strings", just they won't
                         * be tagged as MSIM_TYPE_STRING (as MSIM_TYPE_STRING is intended to be used
                         * by msimprpl code for things like instant messages - stuff that should be
                         * escaped if needed). DWIM.
                         */

                        /* Previously, incoming messages were stored as MSIM_TYPE_STRING.
                         * This was fine for integers and strings, since they can easily be
                         * converted in msim_get_*, as desirable. However, it does not work
                         * well for binary strings. Consider:
                         *
                         * If incoming base64'd elements were tagged as MSIM_TYPE_STRING.
                         * msim_msg_get_binary() sees MSIM_TYPE_STRING, base64 decodes, returns.
                         * everything is fine.
                         * But then, msim_send() is called on the incoming message, which has
                         * a base64'd MSIM_TYPE_STRING that really is encoded binary. The values
                         * will be escaped since strings are escaped, and / becomes /2; no good.
                         *
                         */
                        *binary_data = (gchar *)purple_base64_decode((const gchar *)elem->data, binary_length);
                        return TRUE;
   ...
}

Which is very simple case of the bug discussed here and it was fixed using the patch below.

 			*binary_data = (gchar *)purple_base64_decode((const gchar *)elem->data, binary_length);
-			return TRUE;
+			return ((*binary_data) != NULL);
 
 		case MSIM_TYPE_BINARY:

And we can move to the next one now…

Oscar Session Starting
Oscar session starting routine which resides at libpurple/protocols/oscar/clientlogin.c includes the following incorrect call:

static void start_oscar_session_cb(PurpleUtilFetchUrlData *url_data, gpointer user_data, const gchar *url_text, gsize len, const gchar *error_message)
{
        OscarData *od;
        PurpleConnection *gc;
        char *host, *cookie;
        char *tls_certname = NULL;
        unsigned short port;
        guint8 *cookiedata;
        gsize cookiedata_len;
   ...
        cookiedata = purple_base64_decode(cookie, &cookiedata_len);
        oscar_connect_to_bos(gc, od, host, port, cookiedata, cookiedata_len, tls_certname);
        g_free(cookiedata);

        g_free(host);
        g_free(cookie);
        g_free(tls_certname);
}

Here the patch was to initialize the cookie data’s length to avoid calling it with some uninitialized value that could trigger the faulty behavior like this:

 	guint8 *cookiedata;
-	gsize cookiedata_len;
+	gsize cookiedata_len = 0;

QQ Instant Messaging Formatting

Which can be seen at libpurple/protocols/qq/im.c:

qq_im_format *qq_im_fmt_new_by_purple(const gchar *msg)
{
        qq_im_format *fmt;
        const gchar *start, *end, *last;
        GData *attribs;
        gchar *tmp;
        unsigned char *rgb;
   ...
        while (purple_markup_find_tag("font", last, &start, &end, &attribs)) {
   ...
                tmp = g_datalist_get_data(&attribs, "color");
                if (tmp && strlen(tmp) > 1) {
                        rgb = purple_base16_decode(tmp + 1, NULL);
                        g_memmove(fmt->rgb, rgb, 3);
                        g_free(rgb);
                }
   ...
        return fmt;
}

The concept is pretty much the same but instead of Base64 it uses Base16 encoding. The patch is simply:

 		tmp = g_datalist_get_data(&attribs, "color");
 		if (tmp && strlen(tmp) > 1) {
-			rgb = purple_base16_decode(tmp + 1, NULL);
-			g_memmove(fmt->rgb, rgb, 3);
+			unsigned char *rgb;
+			gsize rgb_len;
+			rgb = purple_base16_decode(tmp + 1, &rgb_len);
+			if (rgb != NULL && rgb_len >= 3)
+				g_memmove(fmt->rgb, rgb, 3);
 			g_free(rgb);
 		}

That adds the missing checks.

Yahoo! Status Processing
You can find this at libpurple/protocols/yahoo/libymsg.c source code file like this:

static void yahoo_process_status(PurpleConnection *gc, struct yahoo_packet *pkt)
{
        PurpleAccount *account = purple_connection_get_account(gc);
        GSList *l = pkt->hash;
        YahooFriend *f = NULL;
        char *name = NULL;
        gboolean unicode = FALSE;
        char *message = NULL;
        YahooFederation fed = YAHOO_FEDERATION_NONE;
        char *fedname = NULL;
   ...
                case 197: /* Avatars */
                {
                        guchar *decoded;
                        char *tmp;
                        gsize len;

                        if (pair->value) {
                                decoded = purple_base64_decode(pair->value, &len);
                                if (len) {
                                        tmp = purple_str_binary_to_ascii(decoded, len);
                                        purple_debug_info("yahoo", "Got key 197, value = %s\n", tmp);
                                        g_free(tmp);
                                }
                                g_free(decoded);
                        }
                        break;
                }
   ...
        g_free(fedname);
}

Which can be reached through avatar parsing and as you might have been expecting, the patch is:

 				decoded = purple_base64_decode(pair->value, &len);
-				if (len) {
+				if (decoded && len > 0) {
 					tmp = purple_str_binary_to_ascii(decoded, len);

To add the NULL check.

Yahoo! P2P Processing
Just like the previous one, this is also part of libpurple/protocols/yahoo/libymsg.c file and you can see a snippet below…

static void yahoo_process_p2p(PurpleConnection *gc, struct yahoo_packet *pkt)
{
        GSList *l = pkt->hash;
        char *who = NULL;
        char *base64 = NULL;
        guchar *decoded;
        gsize len;
   ...
        if (base64) {
                guint32 ip;
                YahooFriend *f;
                char *host_ip;
                struct yahoo_p2p_data *p2p_data;

                decoded = purple_base64_decode(base64, &len);
                if (len) {
                        char *tmp = purple_str_binary_to_ascii(decoded, len);
                        purple_debug_info("yahoo", "Got P2P service packet (from server): who = %s, ip = %s\n", who, tmp);
                        g_free(tmp);
                }
   ...
}

That can be triggered when Base64 messages are being used and it was patched like this:

		YahooFriend *f;
-		char *host_ip;
+		char *host_ip, *tmp;
 		struct yahoo_p2p_data *p2p_data;
 
 		decoded = purple_base64_decode(base64, &len);
-		if (len) {
-			char *tmp = purple_str_binary_to_ascii(decoded, len);
-			purple_debug_info("yahoo", "Got P2P service packet (from server): who = %s, ip = %s\n", who, tmp);
-			g_free(tmp);
+		if (decoded == NULL) {
+			purple_debug_info("yahoo","p2p: Unable to decode base64 IP (%s) \n", base64);
+			return;
 		}
+		tmp = purple_str_binary_to_ascii(decoded, len);
+		purple_debug_info("yahoo", "Got P2P service packet (from server): who = %s, ip = %s\n", who, tmp);
+		g_free(tmp);
 
 		ip = strtol((gchar *)decoded, NULL, 10);

Jabber MD5 Authentication
The last one can be found at libpurple/protocols/jabber/auth_digest_md5.c in the following code:

static JabberSaslState
digest_md5_handle_challenge(JabberStream *js, xmlnode *packet,
                            xmlnode **response, char **msg)
{
        xmlnode *reply = NULL;
        char *enc_in = xmlnode_get_data(packet);
        char *dec_in;
        char *enc_out;
        GHashTable *parts;
        JabberSaslState state = JABBER_SASL_STATE_CONTINUE;
   ...
        dec_in = (char *)purple_base64_decode(enc_in, NULL);
        purple_debug_misc("jabber", "decoded challenge (%"
                        G_GSIZE_FORMAT "): %s\n", strlen(dec_in), dec_in);

        parts = parse_challenge(dec_in);
   ...
        return state;
}

Which was also patched using a similar approach…

 	purple_debug_misc("jabber", "decoded challenge (%"
-			G_GSIZE_FORMAT "): %s\n", strlen(dec_in), dec_in);
+			G_GSIZE_FORMAT "): %s\n",
+			dec_in != NULL ? strlen(dec_in) : 0,
+			dec_in != NULL  ? dec_in : "(null)");
 
 	parts = parse_challenge(dec_in);

Written by xorl

November 15, 2010 at 21:04

Posted in bugs

3 Responses

Subscribe to comments with RSS.

  1. g_malloc0(), unlike its malloc() brethren, will result in a process abort rather than ever return NULL. Please correct me if I’m mistaken (otherwise I’ve written a lot of buggy code this year!)

    David

    David W

    November 15, 2010 at 21:28

  2. That’s right. It will only return NULL if zero size is requested.

    gpointer
    g_malloc0 (gsize n_bytes)
    {
      if (G_UNLIKELY (!g_mem_initialized))
        g_mem_init_nomessage();
      if (G_LIKELY (n_bytes))
        {
     ...
        }
    
      TRACE(GLIB_MEM_ALLOC((void*) NULL, (int) n_bytes, 1, 0));
    
      return NULL;
    }
    

    xorl

    November 15, 2010 at 21:50

  3. Ah, of course. :) Thanks

    David W

    November 15, 2010 at 21:58


Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s