[PATCH] Fix for silc client multiple heap corruption bugs + work to prevent refcount overflow in client + fix for null deref when leaving channel with private key set

Skywing Skywing at valhallalegends.com
Sat Apr 19 18:15:21 CEST 2008


Hi,

This patch fixes a number of silc client bugs that I have encountered:

- Sometimes, silcd seems to send a duplicate notify packet for notify client signoff when there are users with the same nick present.  I assume that this is a silcd bug, but regardless, the end effect of this condition is that the silc client will try to use a SilcClientEntry object after deletion.  The client should never corrupt the heap because of the server sending messages in the wrong state, so this fixes that bug.  (This has historically been the cause for most of my "silc randomly dies with segfault after running for a long time" problems.)
- I have also fixed a similar bug with channel deletion in the silc client, and although I have not seen the server get into a broken state and send duplicate messages that would cause a channel to be attempted to be deleted twice, a malicious server could do so and cause heap corruption clientside.  This has been fixed.
- I have fixed a null dereference that occurs when leaving a channel with a private key (also often happens when disconnecting from a server after having been in a channel with a private key, depending on how graceful the disconnect is).  This problem occurred because by the time we delete any private keys attached to a channel, the cipher information for the channel is already gone, and we assume that it is still valid.
- The silc client uses 16-bit and even 8-bit atomic integers for refcounts in places.  This is unsafe, I think, as it places us in danger of a malicious server overflowing a reference count and causing heap corruption (heap use after free).  I have changed the silc client to use 32-bit integers for reference counts universally.  Command IDs are kept as 16-bit sequence numbers.

There are still some outstanding bugs I that haven't dealt with.  The silc FSM logic seems to either be subtlety broken or something is using it wrong as, particularly when running many clients within the same silc process and connecting and disconnecting them repeatedly, I'll often get asserts about threadcount going to zero.  I'm hoping that somebody who has already more familiar with the FSM logic might be willing to take a look at some of those crashes first.

Also, there still exists a bug where a timed out key exchange will leave a prompt up that will crash your client if you respond to it (bad).  Furthermore, if you attempt to do a "/key negotiate user address_of_silc_server 706" (obviously intended to be run against a client and not a server), silc will blow up in interesting ways, leading me to believe that the client to client key exchange logic is a bit flaky as well.

- S
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.silcnet.org/pipermail/silc-devel/attachments/20080419/80611ee2/attachment-0001.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: client.patch
Type: application/octet-stream
Size: 21351 bytes
Desc: client.patch
Url : http://lists.silcnet.org/pipermail/silc-devel/attachments/20080419/80611ee2/client-0001.obj


More information about the silc-devel mailing list