Subject: Re: [libssh2] Crash while Key Exchange at libssh2 session startup time!!

Re: [libssh2] Crash while Key Exchange at libssh2 session startup time!!

From: James Housley <jim_at_thehousleys.net>
Date: Tue, 7 Nov 2006 20:07:51 -0500

On Nov 7, 2006, at 6:22 PM, Chris Nystrom wrote:

> On 11/7/06, Satish Mittal <satish.mittal_at_gmail.com> wrote:
>> Dear All,
>>
>> I am using libssh2-0.14 on windows. From my application, when i call
>> libssh2_session_startup(), my application is randomly (but very
>> frequently)
>> crashing, reporting 'out of virtual memory'.
>>
>> The problem is during the DH key exchange phase, when the call to
>> libssh2_packet_read() is made,
>>
>> long buf_len, payload_len; //line 876
>> unsigned long packet_length;
>> unsigned long padding_length;
>>
>> if (should_block) {
>> buf_len = libssh2_blocking_read(session, buf, 5);

This should also have a check of the return value for the same reasons.

                   tmpvar = libssh2_blocking_read(session, buf, 5 -
buf_len);

                   if (tmpvar < 0) {
                       /* something bad happened */
                       return(-1);
                   }
                  buf_len = tmpvar;

>> } else {
>> buf_len = recv(session->socket_fd, buf, 1,
>> LIBSSH2_SOCKET_RECV_FLAGS(session));
>> if (buf_len <= 0) {
>> return 0;
>> }
>> buf_len += libssh2_blocking_read(session, buf, 5 -
>> buf_len);
>> //line 887
>
> How about replacing the above line with something like:
>
> tmpvar = libssh2_blocking_read(session, buf, 5 -
> buf_len);
>
> if (tmpvar < 0) {
> /* something bad happened */
> return(-1);
> }
>
> buf_len += tmpvar;
>
> It does no good for functions to provide error codes if you do not
> check for them. :)
>
>> }
>> if (buf_len < 5) {
>> /* Something bad happened */
>> return -1; //line 891
>> }
>> packet_length = libssh2_ntohu32(buf); //line893
>> padding_length = buf[4];
>> #ifdef LIBSSH2_DEBUG_TRANSPORT
>> _libssh2_debug(session, LIBSSH2_DBG_TRANS, "Processing
>> plaintext packet
>> %lu bytes long (with %lu bytes padding)", packet_length,
>> padding_length);
>> #endif
>>
>> payload_len = packet_length - padding_length - 1; /*
>> padding_length(1) */ //line 899
>> payload = LIBSSH2_ALLOC(session, payload_len);
>>
>> Here payload_len is coming out to be a huge number (junk value on
>> stack)
>> since packet_length in line 893 is junk. This is in turn because
>> in line
>> 887, the libssh2_blocking_read() returns -1. But since buf_len is
>> declared
>> as an unsigned long, so -1 gets converted to a huge 32bit value,
>> which then
>> eventually crashes the application.
>>
>> I have tried editing line 876 to declare buf_len and payload_len
>> as long,
>> instead of unsigned long. This then in turn returns -1 in line 891
>> and fails
>> the libssh2_session_startup() call.
>

Jim

--
/"\   ASCII Ribbon Campaign  .
\ / - NO HTML/RTF in e-mail  .
  X  - NO Word docs in e-mail .
/ \ -----------------------------------------------------------------
jeh@FreeBSD.org      http://www.FreeBSD.org     The Power to Serve
jim@TheHousleys.Net  http://www.TheHousleys.net
---------------------------------------------------------------------
Do not meddle in the affairs of dragons, for you are crunchy and taste
     good with ketchup.
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
libssh2-devel mailing list
libssh2-devel_at_lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libssh2-devel
Received on 2006-11-08