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-develReceived on 2014-12-24