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_at_lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libssh2-devel
Received on 2009-06-30