[Pidgin] #14636: Heap memory corruption using g_markup_escape_text() without sanitizing first

[Pidgin]

#14636: Heap memory corruption using g_markup_escape_text() without sanitizing
first
------------------------------------------------+---------------------------
 Reporter:  dbauche                             |     Owner:  elb   
     Type:  defect                              |    Status:  new   
Component:  SILC                                |   Version:  2.10.0
 Keywords:  Overflow,Heap,g_markup_escape_text  |  
------------------------------------------------+---------------------------
 Can anyone confirm if there is no ticket for this yet?

 From libpurple/protocols/silc/ops.c:
 --------
 static void
 silc_private_message(SilcClient client, SilcClientConnection conn,
              SilcClientEntry sender, SilcMessagePayload payload,
              SilcMessageFlags flags, const unsigned char *message,
              SilcUInt32 message_len)
 {
     PurpleConnection *gc = client->application;
     SilcPurple sg = gc->proto_data;
     PurpleConversation *convo = NULL;
     char *msg, *tmp;

     [...]

     if (flags & SILC_MESSAGE_FLAG_UTF8) {
     tmp = g_markup_escape_text((const char *)message, -1);
     /* Send to Purple */
     serv_got_im(gc, sender->nickname, tmp, 0, time(NULL));
     g_free(tmp);

     [...]
 }
 --------
 silc_private_message() and silc_channel_message() both take the "flags"
 argument, which are sent by the client along with the message.

 Inside of this function, we can read that if the SILC_MESSAGE_FLAG_UTF8
 flag is set, g_markup_escape_text() is called.
 g_markup_escape_text() in turn calls append_escaped_text() as seen here:

 From glib/gmarkup.c:
 --------
 gchar*
 g_markup_escape_text (const gchar *text,
                       gssize       length)
 {
   GString *str;

   g_return_val_if_fail (text != NULL, NULL);

   if (length < 0)
     length = strlen (text);

   /* prealloc at least as long as original text */
   str = g_string_sized_new (length);
   append_escaped_text (str, text, length);

   return g_string_free (str, FALSE);
 }

 [...]

 static void
 append_escaped_text (GString     *str,
                      const gchar *text,
                      gssize       length)
 {
   const gchar *p;
   const gchar *end;
   gunichar c;

   p = text;
   end = text + length;

   while (p != end)
     {
       const gchar *next;
       next = g_utf8_next_char (p);

   [...]
       p = next;
     }
 }

 --------
 As seen above, it enters a loop calling g_utf8_next_char() with our user-
 controlled message until "next" matches "end".

 Doing a quick search you can find the documentation for
 g_utf8_next_char():
 --------
 #define g_utf8_next_char(p) (char *)((p) + g_utf8_skip[*(const guchar
 *)(p)])

 "Skips to the next character in a UTF-8 string. The string must be valid;
 this macro is as fast as possible,
 and has no error-checking. You would use this macro to iterate over a
 string character by character.
 The macro returns the start of the next UTF-8 character. Before using this
 macro, use g_utf8_validate()
 to validate strings that may contain invalid UTF-8."
 --------

 The documentation says we need to validate the string as valid UTF-8 first
 before using g_utf8_next_char(). but this code never does it.

 This in turn makes g_utf_next_char() return invalid pointers if the
 message is not valid utf-8, which in turn means the while(p != end)
 condition
 is never met, taking the processor into an infinite loop with p increasing
 until it tries to read non-segmented memory:

 Finally, we get a pidgin crash when using the SILC plugin and sending a
 channel or private message (Sending a message like "\x61\xf8" does the
 trick):
 --------
 Program received signal SIGSEGV, Segmentation fault.
 append_escaped_text (text=0x845a000 <Address 0x845a000 out of bounds>,
 length=-1)
     at /build/buildd-
 glib2.0_2.24.2-1-i386-AScyie/glib2.0-2.24.2/glib/gmarkup.c:2040
 2040    /build/buildd-
 glib2.0_2.24.2-1-i386-AScyie/glib2.0-2.24.2/glib/gmarkup.c: No such file
 or directory.
     in /build/buildd-
 glib2.0_2.24.2-1-i386-AScyie/glib2.0-2.24.2/glib/gmarkup.c
 (gdb) x/i $pc
 0x471108 <IA__g_markup_escape_text+120>:        movzx  eax,BYTE PTR [esi]
 (gdb) x/x $esi
 0x845a000: Cannot access memory at address 0x845a000
 (gdb)
 --------

 I'm not sure if this is bug is only found inside the SILC plugin, I did
 not check other protocols, but anything using g_markup_escape_text()
 without making sure it is proper UTF-8 is potentially susceptible to the
 same problems.

 Wouldn't adding something like g_utf8_validate(message, -1,
 message+strlen(message)); before calling g_markup_escape_text() fix this?

 Seems to be present on the latest releases of pidgin (And in theory, i'm
 not sure... on all clients using libpurple with the same glib2 libs).
 Tested on Windows and Linux.

-- 
Ticket URL: <http://developer.pidgin.im/ticket/14636>
Pidgin <http://pidgin.im>
Pidgin