[Pidgin] #16536: crash: double free or corruption

[Pidgin]

#16536: crash: double free or corruption
------------------------------------+------------------------
 Reporter:  jts                     |       Owner:
     Type:  patch                   |      Status:  new
Milestone:  Patches Needing Review  |   Component:  libpurple
  Version:  2.10.11                 |  Resolution:
 Keywords:                          |
------------------------------------+------------------------
Changes (by Robby):

 * milestone:   => Patches Needing Review


Old description:

> pidgin crashes when disabling a SIP account:
>
> {{{
> *** glibc detected *** /home/eda/tools/vendors/pidgin/2.10.11/bin/pidgin:
> double free or corruption (fasttop): 0x0000000000cccca0 ***
> }}}
>
> Backtrace:
>
> {{{
> #5  0x00002aaaad0038cf in g_free (mem=0xcccca0) at gmem.c:252
> #6  0x00002aaaad3300e6 in purple_proxy_connect_data_destroy
> (connect_data=0xcccc00) at proxy.c:586
> #7  0x00002aaab5289b00 in sipe_backend_transport_disconnect
> (conn=0xcc8e60) at purple-transport.c:324
> #8  0x00002aaab525a4ba in sip_transport_disconnect
> (sipe_private=0xd449a0) at sip-transport.c:1385
> #9  0x00002aaab5264f5b in sipe_core_connection_cleanup
> (sipe_private=0xd449a0) at sipe-core.c:336
> #10 0x00002aaab5265063 in sipe_core_deallocate (sipe_public=<value
> optimized out>) at sipe-core.c:395
> #11 0x00002aaab5253ce6 in sipe_purple_close (gc=0xc9b010) at purple-
> plugin.c:485
> #12 0x00002aaaad31167d in _purple_connection_destroy (gc=0xc9b010) at
> connection.c:275
> #13 0x00002aaaad300e22 in purple_account_disconnect (account=0x8b7650) at
> account.c:1343
> }}}
>
> The problem starts in {{{_purple_connection_destroy()}}}:
>
> {{{
>     purple_proxy_connect_cancel_with_handle(gc);
>
>     prpl_info = PURPLE_PLUGIN_PROTOCOL_INFO(gc->prpl);
>     if (prpl_info->close)
>       (prpl_info->close)(gc);
> }}}
>
> {{{purple_proxy_connect_cancel_with_handle()}}} starts a sequence which
> calls {{{purple_proxy_connect_data_destroy()}}}, which frees
> {{{connect_data}}}. {{{prpl_info->close}}} goes through the sequence
> shown in the backtrace, which ends up at
> {{{purple_proxy_connect_data_destroy()}}} again, resulting in a double
> free.
>
> The following patch appears to fix the problem:
>
> {{{
> *** proxy.c     2015-01-16 01:40:32.000000000 -0500
> --- proxy.c.new 2015-01-16 01:39:46.000000000 -0500
> ***************
> *** 568,573 ****
> --- 568,576 ----
>   static void
>   purple_proxy_connect_data_destroy(PurpleProxyConnectData *connect_data)
>   {
> +       if (!PURPLE_PROXY_CONNECT_DATA_IS_VALID(connect_data))
> +               return;
> +
>         handles = g_slist_remove(handles, connect_data);
>
>         if (connect_data->query_data != NULL)
> }}}
>
> You could argue the problem is in the SIPE plugin instead of pidgin, but
> since all {{{purple_proxy_connect_data_destroy()}}} does is work on the
> {{{connect_data}}} argument, it seems an ounce of prevention is worth a
> pound of cure by verifying it's valid first. However, if it's better
> dealt with in the plugin code just let me know, and I'll file a ticket
> with the SIPE project. Thanks.

New description:

 pidgin crashes when disabling an SIP account:

 {{{
 *** glibc detected *** /home/eda/tools/vendors/pidgin/2.10.11/bin/pidgin:
 double free or corruption (fasttop): 0x0000000000cccca0 ***
 }}}

 Backtrace:

 {{{
 #5  0x00002aaaad0038cf in g_free (mem=0xcccca0) at gmem.c:252
 #6  0x00002aaaad3300e6 in purple_proxy_connect_data_destroy
 (connect_data=0xcccc00) at proxy.c:586
 #7  0x00002aaab5289b00 in sipe_backend_transport_disconnect
 (conn=0xcc8e60) at purple-transport.c:324
 #8  0x00002aaab525a4ba in sip_transport_disconnect (sipe_private=0xd449a0)
 at sip-transport.c:1385
 #9  0x00002aaab5264f5b in sipe_core_connection_cleanup
 (sipe_private=0xd449a0) at sipe-core.c:336
 #10 0x00002aaab5265063 in sipe_core_deallocate (sipe_public=<value
 optimized out>) at sipe-core.c:395
 #11 0x00002aaab5253ce6 in sipe_purple_close (gc=0xc9b010) at purple-
 plugin.c:485
 #12 0x00002aaaad31167d in _purple_connection_destroy (gc=0xc9b010) at
 connection.c:275
 #13 0x00002aaaad300e22 in purple_account_disconnect (account=0x8b7650) at
 account.c:1343
 }}}

 The problem starts in {{{_purple_connection_destroy()}}}:

 {{{
     purple_proxy_connect_cancel_with_handle(gc);

     prpl_info = PURPLE_PLUGIN_PROTOCOL_INFO(gc->prpl);
     if (prpl_info->close)
       (prpl_info->close)(gc);
 }}}

 {{{purple_proxy_connect_cancel_with_handle()}}} starts a sequence which
 calls {{{purple_proxy_connect_data_destroy()}}}, which frees
 {{{connect_data}}}. {{{prpl_info->close}}} goes through the sequence shown
 in the backtrace, which ends up at
 {{{purple_proxy_connect_data_destroy()}}} again, resulting in a double
 free.

 The following patch appears to fix the problem:

 {{{
 *** proxy.c     2015-01-16 01:40:32.000000000 -0500
 --- proxy.c.new 2015-01-16 01:39:46.000000000 -0500
 ***************
 *** 568,573 ****
 --- 568,576 ----
   static void
   purple_proxy_connect_data_destroy(PurpleProxyConnectData *connect_data)
   {
 +       if (!PURPLE_PROXY_CONNECT_DATA_IS_VALID(connect_data))
 +               return;
 +
         handles = g_slist_remove(handles, connect_data);

         if (connect_data->query_data != NULL)
 }}}

 You could argue the problem is in the SIPE plugin instead of pidgin, but
 since all {{{purple_proxy_connect_data_destroy()}}} does is work on the
 {{{connect_data}}} argument, it seems an ounce of prevention is worth a
 pound of cure by verifying it's valid first. However, if it's better dealt
 with in the plugin code just let me know, and I'll file a ticket with the
 SIPE project. Thanks.

--

--
Ticket URL: <https://developer.pidgin.im/ticket/16536#comment:1>
Pidgin <https://pidgin.im>
Pidgin