Re: [PATCH] In handle_channel_request_open(), variable type is freed too early and cause memory corruptions.

[Vic Lee <llyzs@xxxxxxx>]

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
> > 
> 
> 
> 
>