Hi Aris, I can almost reliably to get a segmentation fault when trying to tunnel an "iceweasel" on my debian system. After the patch the problem is gone. I think we do need a security release for v0-4 to fix this issue.
Thanks, Vic On Wed, 2010-01-20 at 13:16 +0800, Vic Lee wrote: > Hi Aris, > > It's hard to describe exactly how to reproduce the double-free but it > did happen to me from time to time. From the source code before the > patch, any "goto error" after "string_free(type);" will always > double-free, because there's another string_free(type) in the error > handler. But it's hard to say when "goto error" will happen. My suspect > is this: > > msg->channel_request_open.originator = string_to_char(type); > > If type is freed, it could point to arbitrary data. The string_to_char > function could try to allocate an unexpected large amount of memory, > fail, then return NULL, and finally lead to double-free in the error > handler. > > Vic > > On Tue, 2010-01-19 at 18:12 +0100, Aris Adamantiadis wrote: > > Vic Lee a écrit : > > > Hi, > > > > > > I recently encounter some occasional crashes when trying to use channel > > > request stuff. It turned out that in function > > > handle_channel_request_open(), a variable 'type' is freed but then used > > > in later codes, which causes unexpected result, sometimes double-freed > > > and crash, but if you are lucky sometimes things won't happen. > > > > > > I will consider this bug a very critical one because this will make the > > > function handle_channel_request_open() very unstable, affecting both > > > server side (all channel requests) and client side (x11 and > > > forward-listen requests) > > > > > > I found this in v0-4 branch, please help me to check master as well... > > > > > > Thanks, > > > > > > Vic > > > > > Thanks for your patch. > > > > I will check on the master branch. I remember having reworked that code > > (it's funny I did not see the problem), maybe I have corrected it. > > Moreover, a double free is a security problem. If it is easy to > > reproduce, we may have to do a security release. > > > > Aris > > > > > >
