From 73e37b3b49587f322d073cf4f7f4fb15bb5f7046 Mon Sep 17 00:00:00 2001 From: Alexander Lamaison Date: Wed, 30 Dec 2009 04:15:28 +0000 Subject: [PATCH] Fixed memory leak in userauth_publickey(). userauth_publickey_fromfile() reads the key from a file using file_read_publickey() which returns two allocated strings, the decoded key and the key method (such as "ssh-dss"). The latter can be derived from the former but returning both avoids a later allocation while doing so. Older versions of userauth_publickey_fromfile() used this method string directly but when userauth_publickey() was factored out of userauth_publickey_fromfile() it derived the method from the key itself. This resulted in the method being allocated twice. This fix, which maintains the optimisation that avoids an extra allocation, changes userauth_publickey() so it doesn't allocate and derive the method when userauth_pblc_method already has a value. Signed-off-by: Alexander Lamaison --- src/userauth.c | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/src/userauth.c b/src/userauth.c index 9e5c491..1343f97 100644 --- a/src/userauth.c +++ b/src/userauth.c @@ -41,6 +41,8 @@ #include #include +#include + /* Needed for struct iovec on some platforms */ #ifdef HAVE_SYS_UIO_H #include @@ -469,7 +471,10 @@ libssh2_userauth_password_ex(LIBSSH2_SESSION *session, const char *username, * * Read a public key from an id_???.pub style file * - * Returns an allocated string in *pubkeydata on success. + * Returns an allocated string containing the decoded key in *pubkeydata + * on success. + * Returns an allocated string containing the key method (e.g. "ssh-dss") + * in method on success. */ static int file_read_publickey(LIBSSH2_SESSION * session, unsigned char **method, @@ -948,16 +953,27 @@ userauth_publickey(LIBSSH2_SESSION *session, memset(&session->userauth_pblc_packet_requirev_state, 0, sizeof(session->userauth_pblc_packet_requirev_state)); - session->userauth_pblc_method_len = _libssh2_ntohu32(pubkeydata); - session->userauth_pblc_method = - LIBSSH2_ALLOC(session, session->userauth_pblc_method_len); + /* + * As an optimisation, userauth_publickey_fromfile reuses a + * previously allocated copy of the method name to avoid an extra + * allocation/free. + * For other uses, we allocate and populate it here. + */ if (!session->userauth_pblc_method) { - libssh2_error(session, LIBSSH2_ERROR_ALLOC, - "Unable to allocate memory for public key data", 0); - return -1; + session->userauth_pblc_method_len = _libssh2_ntohu32(pubkeydata); + session->userauth_pblc_method = + LIBSSH2_ALLOC(session, session->userauth_pblc_method_len); + if (!session->userauth_pblc_method) { + libssh2_error(session, LIBSSH2_ERROR_ALLOC, + "Unable to allocate memory for public key " + "data", 0); + return -1; + } + memcpy(session->userauth_pblc_method, pubkeydata + 4, + session->userauth_pblc_method_len); } - memcpy(session->userauth_pblc_method, pubkeydata + 4, - session->userauth_pblc_method_len); + assert( /* preallocated method len should match what we expect */ + session->userauth_pblc_method_len == _libssh2_ntohu32(pubkeydata)); /* * 45 = packet_type(1) + username_len(4) + servicename_len(4) +