www.libssh2.org | Daily snapshots | Mailing list archive | Docs | Examples | github

Archive Index This month's Index

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:05:52 +0200

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?

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

the libssh2 team