This mailing list has been abandoned! Subscribe to and use the new list instead: http://cool.haxx.se/mailman/listinfo/libssh2-devel ----------------------------------------------------------------------
Bugs item #2814613, was opened at 2009-06-30 14:04 Message generated for change (Comment added) made by fdupoux You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=703942&aid=2814613&group_id=125852 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: misc Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Francois Dupoux (fdupoux) Assigned to: Nobody/Anonymous (nobody) Summary: _libssh2_transport_read hangs when invalid packet received Initial Comment: Hi, I am developping an application which is making intensive use of libssh2. It works well 99% of the time but after having be running successfully several hours/days the application suddenly hangs. The stacktrace I get from gdb says the problem is in _libssh2_transport_read(). The cpu is spinning (100% used) because there is an infinite loop in this function. After investigating I found that the packet_lenght is invalid (negative number). Since many other variables are calculated from packet_lenght, they are all wrong, and the "do { ... } while (loop)" hangs. I don't know how we end up with an invalid packet. This should not happen because of the TCP checksumming. I wonder if it's due to a memory corruption somewhere in the application of if the original packet has been corrupted upstream. Anyway, I think a trivial test should be added to allow only packet with a valid packet_length to be processed. I guess the minimal size is 8, and the maximum packet size is probably 16K. I don't know what the correct action is: should we discard this message, or should the connection be closed ? Here are the line that gets the packet size: p->packet_length = _libssh2_ntohu32(block); We should probably check that padding_length is valid as well. p->padding_length = block[4]; It may be interesting to see what is done when such a packet is received in openssh sources (packet.c): static u_int packet_length = 0; ...... packet_length = get_u32(cp); if (packet_length < 1 + 4 || packet_length > PACKET_MAX_SIZE) { logit("Bad packet length %u.", packet_length); packet_start_discard(enc, mac, packet_length, PACKET_MAX_SIZE); return SSH_MSG_NONE; } ...... I have seen this bug on an x86_64 Linux operating system (Fedora-8 64bit - 2.6.24.7-92.fc8) with both libssh2-1.1 and libssh2-snapshot-20090623. I had that problem multiples times. In the backtrace, it always hangs on _libssh2_transport_read(), and that function can be called in different contexts. (gdb) thread 4 [Switching to thread 4 (Thread 1105209680 (LWP 6540))]#0 0x00002aaaaad1ef67 in _libssh2_transport_read (session=0x2aaaac1ba630) at transport.c:471 471 remainpack = p->total_num - p->data_num; (gdb) bt #0 0x00002aaaaad1ef67 in _libssh2_transport_read (session=0x2aaaac1ba630) at transport.c:471 #1 0x00002aaaaad08d78 in libssh2_channel_read_ex (channel=0x2aaaac216be0, stream_id=0, buf=0x41d9eb80 "", buflen=<value optimized out>) at channel.c:1766 (gdb) bt #0 0xb7ea6d90 in _libssh2_transport_read (session=0x80aca70) at transport.c:327 #1 0xb7e8a33c in channel_read (channel=0x80a34b8, stream_id=0, buf=0xb72bdbc0 "", buflen=8191) at channel.c:1766 #2 0xb7e8a744 in libssh2_channel_read_ex (channel=0x80a34b8, stream_id=0, buf=0xb72bdbc0 "", buflen=8191) at channel.c:1934 Thanks ---------------------------------------------------------------------- >Comment By: Francois Dupoux (fdupoux) Date: 2009-06-30 16:47 Message: Here is a patch that does quick checks on the sizes. I am not sure what the min and max sizes must be, but it will at least avoid the situation where the sizes are negative. diff -urN libssh2-1.1/src/transport.c libssh2-1.1-fix/src/transport.c --- libssh2-1.1/src/transport.c 2009-03-27 21:57:47.000000000 +0100 +++ libssh2-1.1-fix/src/transport.c 2009-06-30 15:32:22.000000000 +0100 @@ -421,8 +421,13 @@ * and we can extract packet and padding length from it */ p->packet_length = _libssh2_ntohu32(block); + if (p->packet_length < 1 || p->packet_length > PACKETBUFSIZE) + return PACKET_FAIL; + p->padding_length = block[4]; - + if (p->padding_length < 0) + return PACKET_FAIL; + /* total_num is the number of bytes following the initial (5 bytes) packet length and padding length fields */ p->total_num = ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=703942&aid=2814613&group_id=125852 ------------------------------------------------------------------------------ _______________________________________________ libssh2-devel mailing list libssh2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/libssh2-devel
