Crashes when connecting to server that requires STARTTLS

Bug #64372 reported by Johan Kiviniemi
8
Affects Status Importance Assigned to Milestone
loudmouth (Ubuntu)
Fix Released
Medium
Telepathy

Bug Description

After loudmouth has connected to the server and sent the initial handshake, just when the reply is being received, connection_connect_cb is called with some strange arguments:

connection_data->fd == 0
connection_data->connection == 0x1
condition == G_IO_OUT

g_io_channel_unix_get_fd (source) still returns the correct fd.

The actual crash happens when connection_data->connection is used as a pointer.

The attached patch contains a work-around for the problem, but the actual cause is still to be fixed.

Revision history for this message
Johan Kiviniemi (ion) wrote :
Revision history for this message
Johan Kiviniemi (ion) wrote :

Attached is output from the test-lm command (from the loudmouth source) with the patch installed.

Revision history for this message
Daniel Holbach (dholbach) wrote :

Thanks for your bug report and your patch, I forwarded the issue upstream: http://developer.imendio.com/issues/browse/LM-62

Changed in loudmouth:
assignee: nobody → telepathy
importance: Undecided → Medium
status: Unconfirmed → Confirmed
Revision history for this message
Daniel Holbach (dholbach) wrote :

Uploaded the workaround:

 loudmouth (1.1.4-0ubuntu2) edgy; urgency=low
 .
   * debian/patches/01-workaround_0_connection_data.patch:
     - workaround connection_data->fd == 0. (works around Malone: #64372),
       thanks a lot Johan Kiviniemi <email address hidden>

I will keep the bug open until it is fixed upstream.

Revision history for this message
Removed by request (removed637280) wrote :

i don't know but seems I still have it while using 1.1.4-0ubuntu3

Revision history for this message
Daniel Holbach (dholbach) wrote :

This should now work in the upcoming 1.3.1 unstable version.

Changed in loudmouth:
status: Confirmed → Fix Committed
Revision history for this message
Robert McQueen (robot101) wrote :

This patch is an ugly hack because it still relies on reading free()'d memory to detect the symptom of the bug - the attached patch should fix the real problem. In some connection failure paths, _lm_connection_failed_with_error was being called and kicking off a second connection attempt (eg falling back to an IPv4 address if IPv6 has failed), but then the source ID was being overwritten, meaning the source wasn't destroyed, so the callback was still being called after connect_data had been freed. I guess at some point in the past _lm_connection_failed_with_error didn't exist or do retries, so zeroing out the source ID was a reasonable thing to do in case of failure. I've also added some asserts which should trigger if my understanding of the problem and solution are wrong, and these don't trigger on my machine in either the failure or success cases. I'd recommend pushing this into updates for any supported distribution you ship with 1.2.x.

Revision history for this message
Loïc Minier (lool) wrote :

I subscribed Kees.

Kees: could you please confirm this is a security issue? If yes, then we probably need to address it in gutsy, probably feisty as well; no idea for older releases; what do you think?

Revision history for this message
Kees Cook (kees) wrote :

Hi, which side of the communication is crashing? (is it the client or the server that crashes?) From the report, it seems like this is the client, in which case I would not consider this a security issue.

Revision history for this message
Robert McQueen (robot101) wrote :

The client crashes, I don't think it's a security risk because it's not a double free, just dereferencing free'd memory, and doesn't /usually/ crash because with the original patch, it finds the invalid memory and returns from the function. It's only worth an update because I think that examining memory which has been freed is obviously wrong and I think it can still crash in some cases; it's not a security problem.

Revision history for this message
Laurent Bigonville (bigon) wrote :

could someone confirm this bug is fixed?

Changed in loudmouth:
status: Fix Committed → Incomplete
Revision history for this message
Daniel T Chen (crimsun) wrote :

Fixed in 8.10 alpha.

Changed in loudmouth:
status: Incomplete → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.