[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: some possible issues
  [Thread Prev] | [Thread Next]
 
 
- Subject: Re: some possible issues
 - From: "Bernhard R. Link" <brlink@xxxxxxxxxx>
 - Reply-to: libssh@xxxxxxxxxx
 - Date: Wed, 18 Nov 2009 10:51:59 +0100
 - To: libssh@xxxxxxxxxx
 
* Andreas Schneider <mail@xxxxxxxxxxxx> [091118 10:36]:
> > 3)
> > channels.c's channel_default_bufferize looks strange. in case of buffer
> > errors channel->std{out,err}_buffer is freed but not set to NULL, which
> > might cause corrupting memory management (double free or writing to
> > free'd memory). I guess it is even thinkable (though I guess not
> > with realistic thinking) that this might be exploitable remotly
> > to get some code executed in some form.
>
> Are you sure that it is not NULL, we use SAFE_FREE() which is a macro which
> sets a pointer always to NULL after freeing it. I think this needs a closer
> look.
channel_default_bufferize has:
|    if (buffer_add_data(channel->stderr_buffer, data, len) < 0) {
|      buffer_free(channel->stderr_buffer);
|      return -1;
|    }
While SAFE_FREE sets the argument NULL and buffer_free uses SAFE_FREE,
buffer_free itself does not seem to be a macro but a regular function.
That means the copy of channe->stderr_buffer that is buffer_free's
buffer argument is NULL at the end of buffer_free. But that does not
change the value channel->stderr_buffer is.
Hochachtungsvoll,
	Bernhard R. Link
-- 
"Never contain programs so few bugs, as when no debugging tools are available!"
	Niklaus Wirth
| some possible issues | "Bernhard R. Link" <brlink@xxxxxxxxxx> | 
| Re: some possible issues | Andreas Schneider <mail@xxxxxxxxxxxx> |