(re-sending because my original e-mail did not reach the mailing list archives)
Hello Alexander,
> Am 24.12.2014 um 18:10 schrieb Alexander Lamaison <swish_at_lammy.co.uk <mailto:swish_at_lammy.co.uk>>:
>
> On 15 December 2014 at 00:32, Marc Hoersken <info_at_marc-hoersken.de <mailto:info_at_marc-hoersken.de>> wrote:
>> - 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.
This is the code path identified by VS2012:
C6011 Dereferencing null pointer
Dereferencing NULL pointer 'session->kex'.
Line 1693: 'session->kex' may be NULL (Enter this branch)
Line 1704: Skip this branch, (assume '<branch condition>' is false)
Line 1721: Skip this branch, (assume '<branch condition>' is false)
Line 1759: Enter this branch, (assume 'rc==0')
Line 1760: Enter this branch, (assume '<branch condition>')
Line 1761: 'session->kex' is dereferenced, but may still be NULL
libssh2 - kex.c (Line 1761)
>> - 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.
C6011 Dereferencing null pointer
Dereferencing NULL pointer 'listen_state->channel'.
Line 129: 'listen_state->channel' may be NULL
Line 131: Skip this branch, (assume '<branch condition>' is false)
Line 206: Enter this branch, (assume '<branch condition>')
Line 209: Skip this branch, (assume 'rc==-37' is false)
Line 211: Skip this branch, (assume 'rc' is false)
Line 219: 'listen_state->channel' is dereferenced, but may still be NULL
libssh2 - packet.c (Line 219)
> 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 <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
You are probably right, but I posted the code paths above anyway.
I guess it won’t hurt to have those additional checks in place in this case.
What do you think?
Best regards,
Marc
P.S.: Merry Christmas!
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Received on 2015-01-02