www.libssh2.org | Daily snapshots | Mailing list archive | Docs | Examples

Archive Index This month's Index

Subject: Re: [libssh2] Memory leak in payload

Re: [libssh2] Memory leak in payload

From: Dan Fandrich <dan_at_coneharvesters.com>
Date: Wed, 14 Mar 2007 17:19:06 -0700

On Wed, Mar 14, 2007 at 11:13:56PM +0100, Daniel Stenberg wrote:
> Dan, does this problem occur when running one of the sample apps against a
> plain standard OpenSSH server or does it require anything particular?

I found it while running a slightly hacked curl modified to do public key
authentication instead of password authentication downloading a small file
from an OpenSSH 3.9p1 server running sftp. I just tried modifying
simple/sftp.c to set auth_pw=0 and valgrind reported a ton of errors,
but most of them to do with OpenSSL.

After a bit more poking around, I discovered that at the end of fullpacket()
is a call to libssh2_packet_add() which passes in p->payload. The packet
in most cases gets added to a list which is freed later, or freed within that
function. However, there are a couple of error paths where it isn't freed.
This patch fixes those:

Index: packet.c
===================================================================
RCS file: /cvsroot/libssh2/libssh2/src/packet.c,v
retrieving revision 1.45
diff -u -r1.45 packet.c
--- packet.c 2 Feb 2007 23:23:37 -0000 1.45
+++ packet.c 15 Mar 2007 00:13:36 -0000
@@ -332,6 +332,7 @@
                                 if (session->ssh_msg_disconnect) {
                                         LIBSSH2_DISCONNECT(session, SSH_DISCONNECT_MAC_ERROR, "Invalid MAC received", sizeof("Invalid MAC received") - 1, "", 0);
                                 }
+ LIBSSH2_FREE(session, data);
                                 return -1;
                         }
                 } else {
@@ -339,6 +340,7 @@
                         if (session->ssh_msg_disconnect) {
                                 LIBSSH2_DISCONNECT(session, SSH_DISCONNECT_MAC_ERROR, "Invalid MAC received", sizeof("Invalid MAC received") - 1, "", 0);
                         }
+ LIBSSH2_FREE(session, data);
                         return -1;
                 }
         }

It turns out that wasn't the problem I was seeing, though. It was the
pointer returned from libssh2_userauth_list() that wasn't being freed from
within curl.

There's one more leak that valgrind complains about, but it's within
libcrypto so it's not clear that it's a libssh2 problem:

==11948==
==11948== 40 bytes in 2 blocks are definitely lost in loss record 1 of 3
==11948== at 0x1B904984: malloc (vg_replace_malloc.c:131)
==11948== by 0x3A74BD: (within /lib/libcrypto.so.0.9.7a)
==11948== by 0x3A7A6E: CRYPTO_malloc (in /lib/libcrypto.so.0.9.7a)
==11948== by 0x3C0B55: BN_new (in /lib/libcrypto.so.0.9.7a)
==11948== by 0x1B92D8DB: _libssh2_dsa_sha1_verify (openssl.c:153)
==11948== by 0x1B91DBA5: libssh2_hostkey_method_ssh_dss_sig_verify (hostkey.c:304)
==11948== by 0x1B91E866: libssh2_kex_method_diffie_hellman_groupGP_sha1_key_exchange (kex.c:288)
==11948== by 0x1B91F640: libssh2_kex_method_diffie_hellman_group14_sha1_key_exchange (kex.c:542)
==11948== by 0x1B9207FC: libssh2_kex_exchange (kex.c:1266)
==11948== by 0x1B925F6D: libssh2_session_startup (session.c:348)
==11948== by 0x8060930: Curl_ssh_connect (ssh.c:316)
==11948== by 0x806CC36: Curl_protocol_connect (url.c:2446)
==11948== by 0x806F48F: SetupConnection (url.c:4049)
==11948== by 0x806F598: Curl_connect (url.c:4118)
==11948== by 0x8079061: Curl_connect_host (transfer.c:2301)
==11948== by 0x8079252: Curl_perform (transfer.c:2391)
==11948==
==11948== LEAK SUMMARY:
==11948== definitely lost: 40 bytes in 2 blocks.
==11948== possibly lost: 0 bytes in 0 blocks.
==11948== still reachable: 1832 bytes in 20 blocks.
==11948== suppressed: 0 bytes in 0 blocks.
==11948== Reachable blocks (those to which a pointer was found) are not shown.
==11948== To see them, rerun with: --show-reachable=yes

I don't know enough about the libcrypto API to figure out whose fault this
is.

>>> Dan

-- 
http://www.MoveAnnouncer.com              The web change of address service
          Let webmasters know that your web site has moved
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
libssh2-devel mailing list
libssh2-devel_at_lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libssh2-devel
Received on 2007-03-15

the libssh2 team