Subject: Re: libssh2 master a8cfc708 channel: fix possible NULL dereference

Re: libssh2 master a8cfc708 channel: fix possible NULL dereference

From: Kamil Dudka <kdudka_at_redhat.com>
Date: Mon, 8 Oct 2012 16:09:56 +0200

On Monday 08 October 2012 16:05:52 Kamil Dudka wrote:
> On Monday 08 October 2012 15:30:31 Peter Stuge wrote:
> > libssh2_at_git.stuge.se wrote:
> > > +++ b/src/channel.c
> > > @@ -1483,10 +1483,11 @@ libssh2_channel_get_exit_signal(LIBSSH2_CHANNEL
> > > *channel, char **langtag,
> > > size_t *langtag_len)
> > > {
> > > - LIBSSH2_SESSION *session = channel->session;
> > > size_t namelen = 0;
> > >
> > > if (channel) {
> > > + LIBSSH2_SESSION *session = channel->session;
> > > +
> > > if (channel->exit_signal) {
> > > namelen = strlen(channel->exit_signal);
> > > if (exitsignal) {
> >
> > I think this fix is wrong. Please look at what happens after the
> > condition.
>
> It does exactly what the comments above the functions suggests, doesn't it?
>
> > Also, I don't think that libssh2 needs to validate programmer input.
> > If someone passes a NULL pointer to a function that is really an
> > error, and they will then have a problem sooner or later anyway.
> >
> > It is much better for libssh2 to crash fast and hard in this case, to
> > have a higher chance that the programmer discovers the error.
>
> I fully agree with your attitude on this. Then we should just the check
> and update the comment above the function, right?

I meant to _remove_ the check and update the comment.

Kamil
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Received on 2012-10-08