There's no way to interrupt proxy_connect() before reaching the connected callback

[dx]

Originally from this bitlbee-steam bug report: ​https://github.com/jgeboski/bitlbee-steam/issues/71

proxy_connect keeps its phb struct for itself, so while "account off" tries to free all structs and disconnect all the event handlers, it can't do anything about phb and phb->inpa (the event tag for gaim_io_connected), and disconnecting an account in the time between proxy_connect() and gaim_io_connected() results in use-after-free of memory *and* file descriptors.

This can also happen "naturally" due to a bad internet connection, as was the case with the original bug report.

Copypasting my observations (only slightly reformatted) below:

---

Decided to try reproducing this one, copied some of the iptables commands from the ​comcast README and then disconnected/connected the account a bunch of times until it happened:

About to send HTTP request:
POST /ISteamWebUserPresenceOAuth/Logoff/v0001 HTTP/1.1
User-Agent: Steam App / BitlBee / 1.2.0
Content-Length: 61
Connection: Close
Accept: */*
Cookie:
Host: api.steampowered.com
Content-Type: application/x-www-form-urlencoded

access_token=XXXXXXXXXX&umqid=XXXXXXXXXX
About to send HTTP request:
POST /ISteamWebUserPresenceOAuth/Logon/v0001 HTTP/1.1
User-Agent: Steam App / BitlBee / 1.2.0
Content-Length: 73
Connection: Close
Accept: */*
Cookie:
Host: api.steampowered.com
Content-Type: application/x-www-form-urlencoded

ui_mode=web&access_token=XXXXXXXXXX&umqid=XXXXXXXXXX
==28981== Invalid write of size 8
==28981==    at 0x56995A4: gnutls_init (in /usr/lib/libgnutls.so.28.41.4)
==28981==    by 0x13FEFE: ssl_connected (ssl_gnutls.c:322)
==28981==    by 0x13DED5: gaim_io_connected (proxy.c:105)
==28981==    by 0x1362E7: gaim_io_invoke (events_glib.c:86)
==28981==    by 0x538891C: g_main_context_dispatch (in /usr/lib/libglib-2.0.so.0.4200.1)
==28981==    by 0x5388CF7: ??? (in /usr/lib/libglib-2.0.so.0.4200.1)
==28981==    by 0x5389021: g_main_loop_run (in /usr/lib/libglib-2.0.so.0.4200.1)
==28981==    by 0x136259: b_main_run (events_glib.c:59)
==28981==    by 0x134052: main (unix.c:170)
==28981==  Address 0x7cf6d90 is 48 bytes inside a block of size 56 free'd
==28981==    at 0x4C2B200: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==28981==    by 0x140373: ssl_disconnect (ssl_gnutls.c:467)
==28981==    by 0x1383BE: http_close (http_client.c:696)
==28981==    by 0x829AEB7: steam_http_req_close (steam-http.c:364)
==28981==    by 0x829B092: steam_http_req_free (steam-http.c:386)
==28981==    by 0x829B119: steam_http_free_reqs (steam-http.c:78)
==28981==    by 0x8297858: steam_logout (steam.c:895)
==28981==    by 0x145945: imc_logout (nogaim.c:401)
==28981==    by 0x143502: bee_free (bee.c:57)
==28981==    by 0x12098C: irc_free (irc.c:254)
==28981==    by 0x53893C2: ??? (in /usr/lib/libglib-2.0.so.0.4200.1)
==28981==    by 0x538891C: g_main_context_dispatch (in /usr/lib/libglib-2.0.so.0.4200.1)
==28981==

And then this one appeared when I closed that bitlbee:

==28981== 21 bytes in 1 blocks are definitely lost in loss record 41 of 158
==28981==    at 0x4C29F90: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==28981==    by 0x538E579: g_malloc (in /usr/lib/libglib-2.0.so.0.4200.1)
==28981==    by 0x53A6E5E: g_strdup (in /usr/lib/libglib-2.0.so.0.4200.1)
==28981==    by 0x13F972: ssl_connect (ssl_gnutls.c:121)
==28981==    by 0x136A55: http_dorequest (http_client.c:49)
==28981==    by 0x829B8C9: steam_http_req_send (steam-http.c:691)
==28981==    by 0x8298ECD: steam_api_req_logoff (steam-api.c:801)
==28981==    by 0x145945: imc_logout (nogaim.c:401)
==28981==    by 0x143502: bee_free (bee.c:57)
==28981==    by 0x12098C: irc_free (irc.c:254)
==28981==    by 0x53893C2: ??? (in /usr/lib/libglib-2.0.so.0.4200.1)
==28981==    by 0x538891C: g_main_context_dispatch (in /usr/lib/libglib-2.0.so.0.4200.1)

Okay, main difference between gholms' error and mine is that mine has a fd != -1 and his is -1.

The problem is the same though, it just fails at different points. ssl_connected gets called when it shouldn't, and:

• Sometimes it tries to call gnutls_init
• Sometimes it tries to call conn->func(conn->data, 0, NULL, cond) signaling that the connection is dead (because fd == -1)
• Smetimes it tries to call some NSS library function
• Or maybe it gets past that and calls conn->func as if data was received and is ready to process.

Any of those conditions is going to segfault sooner or later because conn and its members are already freed at that point.

In my case, it's because a fd was reused because i reconnected before ssl_connected got called. In gholms', getsockopt probably detected the error condition.

The initial connection flow with no proxy is:

1. ssl_connect(), passing callback ssl_connected
2. proxy_connect(), creating struct PHB for internal connection use
3. proxy_connect_none()
4. b_input_add of gaim_io_connected, setting input tag to phb->inpa
5. gaim_io_connected()
6. phb->func(phb->data, source, B_EV_IO_READ);, where phb->func is ssl_connected
7. ssl_connected()

Disconnect and/or replace the connection between steps 4 and 5 to get this bug.

The struct PHB is internal, created in step 2 and deleted in step 6, never exposed. I see no easy way to clean it up.

This is most likely easier to reproduce with the glib event loop. With libevent it may not crash but it might leak memory (just guessing though), see comments in closesocket() in events_libevent.c (the glib counterpart just calls close(fd). also event_debug is a no-op macro)

[dx]

Fixed with this: ​https://github.com/bitlbee/bitlbee/pull/54

Mainly 4e365cec5275e3dec782af3ec0bc9a651cc2b831 and 0db66186f49438ae9c2f73ca85b915e999896309