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

Archive Index This month's Index

Subject: Re: Additional questions related to my fixes of possible NULL pointer de-references

Re: Additional questions related to my fixes of possible NULL pointer de-references

From: Alexander Lamaison <swish_at_lammy.co.uk>
Date: Wed, 24 Dec 2014 17:10:07 +0000

On 15 December 2014 at 00:32, Marc Hoersken <info_at_marc-hoersken.de> wrote:
> Hello everyone,
>
> I just posted a bunch of patches to the Git repository that are the
> result of running the code analysis feature of VS2012 against libssh2
> using the new CMake generated project files.
>
> Most of them are quite basic, but at least the following two patches
> raise additional questions that I would like to bring to your attention:
> - kex.c: fix possible NULL pointer de-reference with session->kex [1]

This extra check isn't necessary because session->kex cannot be NULL
if rc == 0. The call to kex_agree_methods on line 1749 returns -1 if
session->kex is not initialised.

> - packet.c: fix possible NULL pointer de-reference within listen_state [2]

This case is less obvious, but the check is also unecessary. It's
impossible for listen_state->channel to be NULL before it is
de-referenced. Any path that arrives at line 219 must have passed
line 142 because listen_state->state is initialised as _state_idle and
the only way to reach line 219 is via the state transition on line
203.

> I think that just catching the possible NULL pointer in those code paths
> is actually not enough to make libssh2 behave correctly.
> In my opinion some kind of error code needs to be raised if such an
> error condition is reached.

I think VS2012 was being a little overzealous. Does it output a
particular path it claims would lead to the unhappy situation?

> [1]
> http://git.libssh2.org/?p=libssh2.git;a=commitdiff;h=1c1699545b0a1114e8ca3e6cd097cc9df1e67201;js=1
> [2]
> http://git.libssh2.org/?p=libssh2.git;a=commitdiff;h=e57f29f8f65c83063fd8f63c88f88830fc269bd6;js=1

Alex

-- 
Swish - Easy SFTP for Windows Explorer (http://www.swish-sftp.org)
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Received on 2014-12-24

the libssh2 team