Crashes when connecting to server that requires STARTTLS

[Johan Kiviniemi]

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.

[Johan Kiviniemi (ion)]

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

[Daniel Holbach (dholbach)]

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

[Daniel Holbach (dholbach)]

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.

[Removed by request (removed637280)]

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

[Daniel Holbach (dholbach)]

This should now work in the upcoming 1.3.1 unstable version.

[Robert McQueen (robot101)]

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.

[Loïc Minier (lool)]

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?

[Kees Cook (kees)]

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.

[Robert McQueen (robot101)]

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.

[Laurent Bigonville (bigon)]

could someone confirm this bug is fixed?

[Daniel T Chen (crimsun)]

Fixed in 8.10 alpha.