diff --git a/NEWS b/NEWS index a7e16d0..c1add74 100644 --- a/NEWS +++ b/NEWS @@ -27,7 +27,8 @@ Version Libgcrypt can now be used instead of OpenSSL if you specify --with-libgcrypt. (Simon Josefsson) - Fixed a memory leak in the packet handling. (Dan Fandrich) + Fixed a memory leak in the packet handling, and better handle out of + memory situations. (Dan Fandrich) Made libssh2 build with OpenSSL 0.9.6. (Dan Fandrich) diff --git a/src/channel.c b/src/channel.c index 77b2dc9..353912e 100644 --- a/src/channel.c +++ b/src/channel.c @@ -233,7 +233,7 @@ libssh2_channel_open_ex(LIBSSH2_SESSION *session, const char *channel_type, libssh2_error(session, LIBSSH2_ERROR_ALLOC, "Unable to allocate temporary space for packet", 0); - return NULL; + goto channel_error; } *(s++) = SSH_MSG_CHANNEL_OPEN; libssh2_htonu32(s, channel_type_len); @@ -958,7 +958,7 @@ LIBSSH2_API unsigned long libssh2_channel_receive_window_adjust(LIBSSH2_CHANNEL unsigned char adjust[9]; /* packet_type(1) + channel(4) + adjustment(4) */ if (!force && (adjustment + channel->adjust_queue < LIBSSH2_CHANNEL_MINADJUST)) { - _libssh2_debug(channel->session, LIBSSH2_DBG_CONN, "Queing %lu bytes for receive window adjustment for channel %lu/%lu", adjustment, channel->local.id, channel->remote.id); + _libssh2_debug(channel->session, LIBSSH2_DBG_CONN, "Queueing %lu bytes for receive window adjustment for channel %lu/%lu", adjustment, channel->local.id, channel->remote.id); channel->adjust_queue += adjustment; return channel->remote.window_size; } @@ -1240,7 +1240,7 @@ int _libssh2_channel_write_ex(LIBSSH2_CHANNEL *channel, rc = libssh2_packet_read(session); if (rc < 0) { - /* Error or EAGAIN occured, disconnect? */ + /* Error or EAGAIN occurred, disconnect? */ return rc; } diff --git a/src/comp.c b/src/comp.c index 52d829c..721227a 100644 --- a/src/comp.c +++ b/src/comp.c @@ -191,6 +191,7 @@ static int libssh2_comp_method_zlib_comp(LIBSSH2_SESSION *session, } if (strm->avail_in) { unsigned long out_ofs = out_maxlen - strm->avail_out; + char *newout; out_maxlen += compress ? (strm->avail_in + 4) : (2 * strm->avail_in); @@ -202,11 +203,13 @@ static int libssh2_comp_method_zlib_comp(LIBSSH2_SESSION *session, return -1; } - out = LIBSSH2_REALLOC(session, out, out_maxlen); - if (!out) { + newout = LIBSSH2_REALLOC(session, out, out_maxlen); + if (!newout) { libssh2_error(session, LIBSSH2_ERROR_ALLOC, "Unable to expand compress/decompression buffer", 0); + LIBSSH2_FREE(session, out); return -1; } + out = newout; strm->next_out = (unsigned char *)out + out_ofs; strm->avail_out += compress ? (strm->avail_in + 4) : (2 * strm->avail_in); } else while (!strm->avail_out) { @@ -214,6 +217,7 @@ static int libssh2_comp_method_zlib_comp(LIBSSH2_SESSION *session, * Or potentially many bytes if it's a decompress */ int grow_size = compress ? 8 : 1024; + char *newout; if (out_maxlen >= (int)payload_limit) { libssh2_error(session, LIBSSH2_ERROR_ZLIB, "Excessive growth in decompression phase", 0); @@ -228,11 +232,13 @@ static int libssh2_comp_method_zlib_comp(LIBSSH2_SESSION *session, out_maxlen += grow_size; strm->avail_out = grow_size; - out = LIBSSH2_REALLOC(session, out, out_maxlen); - if (!out) { + newout = LIBSSH2_REALLOC(session, out, out_maxlen); + if (!newout) { libssh2_error(session, LIBSSH2_ERROR_ALLOC, "Unable to expand final compress/decompress buffer", 0); + LIBSSH2_FREE(session, out); return -1; } + out = newout; strm->next_out = (unsigned char *)out + out_maxlen - grow_size; diff --git a/src/kex.c b/src/kex.c index 0e9290c..28aeb2d 100644 --- a/src/kex.c +++ b/src/kex.c @@ -44,21 +44,22 @@ libssh2_sha1_ctx hash; \ unsigned long len = 0; \ if (!(value)) { \ - value = LIBSSH2_ALLOC(session, reqlen + SHA_DIGEST_LENGTH); \ - } \ - while (len < reqlen) { \ - libssh2_sha1_init(&hash); \ - libssh2_sha1_update(hash, k_value, k_value_len); \ - libssh2_sha1_update(hash, h_sig_comp, SHA_DIGEST_LENGTH); \ - if (len > 0) { \ - libssh2_sha1_update(hash, value, len); \ - } else { \ - libssh2_sha1_update(hash, (version), 1); \ - libssh2_sha1_update(hash, session->session_id, session->session_id_len); \ - } \ - libssh2_sha1_final(hash, (value) + len); \ - len += SHA_DIGEST_LENGTH; \ - } \ + value = LIBSSH2_ALLOC(session, reqlen + SHA_DIGEST_LENGTH); \ + } \ + if (value) \ + while (len < reqlen) { \ + libssh2_sha1_init(&hash); \ + libssh2_sha1_update(hash, k_value, k_value_len); \ + libssh2_sha1_update(hash, h_sig_comp, SHA_DIGEST_LENGTH); \ + if (len > 0) { \ + libssh2_sha1_update(hash, value, len); \ + } else { \ + libssh2_sha1_update(hash, (version), 1); \ + libssh2_sha1_update(hash, session->session_id, session->session_id_len); \ + } \ + libssh2_sha1_final(hash, (value) + len); \ + len += SHA_DIGEST_LENGTH; \ + } \ } /* {{{ libssh2_kex_method_diffie_hellman_groupGP_sha1_key_exchange @@ -334,7 +335,16 @@ static int libssh2_kex_method_diffie_hellman_groupGP_sha1_key_exchange(LIBSSH2_S int free_iv = 0, free_secret = 0; LIBSSH2_KEX_METHOD_DIFFIE_HELLMAN_SHA1_HASH(iv, session->local.crypt->iv_len, "A"); + if (!iv) { + ret = -1; + goto clean_exit; + } LIBSSH2_KEX_METHOD_DIFFIE_HELLMAN_SHA1_HASH(secret, session->local.crypt->secret_len, "C"); + if (!secret) { + LIBSSH2_FREE(session, iv); + ret = -1; + goto clean_exit; + } if (session->local.crypt->init(session, session->local.crypt, iv, &free_iv, secret, &free_secret, 1, &session->local.crypt_abstract)) { LIBSSH2_FREE(session, iv); LIBSSH2_FREE(session, secret); @@ -365,7 +375,16 @@ static int libssh2_kex_method_diffie_hellman_groupGP_sha1_key_exchange(LIBSSH2_S int free_iv = 0, free_secret = 0; LIBSSH2_KEX_METHOD_DIFFIE_HELLMAN_SHA1_HASH(iv, session->remote.crypt->iv_len, "B"); + if (!iv) { + ret = -1; + goto clean_exit; + } LIBSSH2_KEX_METHOD_DIFFIE_HELLMAN_SHA1_HASH(secret, session->remote.crypt->secret_len, "D"); + if (!secret) { + LIBSSH2_FREE(session, iv); + ret = -1; + goto clean_exit; + } if (session->remote.crypt->init(session, session->remote.crypt, iv, &free_iv, secret, &free_secret, 0, &session->remote.crypt_abstract)) { LIBSSH2_FREE(session, iv); LIBSSH2_FREE(session, secret); @@ -394,6 +413,10 @@ static int libssh2_kex_method_diffie_hellman_groupGP_sha1_key_exchange(LIBSSH2_S int free_key = 0; LIBSSH2_KEX_METHOD_DIFFIE_HELLMAN_SHA1_HASH(key, session->local.mac->key_len, "E"); + if (!key) { + ret = -1; + goto clean_exit; + } session->local.mac->init(session, key, &free_key, &session->local.mac_abstract); if (free_key) { @@ -412,6 +435,10 @@ static int libssh2_kex_method_diffie_hellman_groupGP_sha1_key_exchange(LIBSSH2_S int free_key = 0; LIBSSH2_KEX_METHOD_DIFFIE_HELLMAN_SHA1_HASH(key, session->remote.mac->key_len, "F"); + if (!key) { + ret = -1; + goto clean_exit; + } session->remote.mac->init(session, key, &free_key, &session->remote.mac_abstract); if (free_key) { @@ -1218,6 +1245,7 @@ int libssh2_kex_exchange(LIBSSH2_SESSION *session, int reexchange) /* session->f { unsigned char *data; unsigned long data_len; + int rc = 0; /* Prevent loop in packet_add() */ session->state |= LIBSSH2_STATE_EXCHANGING_KEYS; @@ -1259,13 +1287,15 @@ int libssh2_kex_exchange(LIBSSH2_SESSION *session, int reexchange) /* session->f session->remote.kexinit_len = data_len; if (libssh2_kex_agree_methods(session, data, data_len)) { - return -3; + rc = -3; } } - if (session->kex->exchange_keys(session)) { - libssh2_error(session, LIBSSH2_ERROR_KEY_EXCHANGE_FAILURE, "Unrecoverable error exchanging keys", 0); - return -4; + if (rc == 0) { + if (session->kex->exchange_keys(session)) { + libssh2_error(session, LIBSSH2_ERROR_KEY_EXCHANGE_FAILURE, "Unrecoverable error exchanging keys", 0); + rc = -4; + } } /* Done with kexinit buffers */ @@ -1280,7 +1310,7 @@ int libssh2_kex_exchange(LIBSSH2_SESSION *session, int reexchange) /* session->f session->state &= ~LIBSSH2_STATE_EXCHANGING_KEYS; - return 0; + return rc; } /* }}} */ diff --git a/src/packet.c b/src/packet.c index ae985de..c7fc124 100644 --- a/src/packet.c +++ b/src/packet.c @@ -368,7 +368,7 @@ int libssh2_packet_add(LIBSSH2_SESSION *session, unsigned char *data, size_t dat if (session->ssh_msg_ignore) { LIBSSH2_IGNORE(session, (char *)data + 4, datalen - 5); } - LIBSSH2_FREE(session, (char *)data); + LIBSSH2_FREE(session, data); return 0; break; case SSH_MSG_DEBUG: @@ -549,6 +549,11 @@ int libssh2_packet_add(LIBSSH2_SESSION *session, unsigned char *data, size_t dat } packet = LIBSSH2_ALLOC(session, sizeof(LIBSSH2_PACKET)); + if (!packet) { + _libssh2_debug(session, LIBSSH2_ERROR_ALLOC, "Unable to allocate memory for LIBSSH2_PACKET"); + LIBSSH2_FREE(session, data); + return -1; + } memset(packet, 0, sizeof(LIBSSH2_PACKET)); packet->data = data; diff --git a/src/publickey.c b/src/publickey.c index 64737c1..d9cd67f 100644 --- a/src/publickey.c +++ b/src/publickey.c @@ -600,13 +600,15 @@ LIBSSH2_API int libssh2_publickey_list_fetch(LIBSSH2_PUBLICKEY *pkey, unsigned l case LIBSSH2_PUBLICKEY_RESPONSE_PUBLICKEY: /* What we want */ if (keys >= max_keys) { + libssh2_publickey_list *newlist; /* Grow the key list if necessary */ max_keys += 8; - list = LIBSSH2_REALLOC(session, list, (max_keys + 1) * sizeof(libssh2_publickey_list)); - if (!list) { + newlist = LIBSSH2_REALLOC(session, list, (max_keys + 1) * sizeof(libssh2_publickey_list)); + if (!newlist) { libssh2_error(session, LIBSSH2_ERROR_ALLOC, "Unable to allocate memory for publickey list", 0); goto err_exit; } + list = newlist; } if (pkey->version == 1) { unsigned long comment_len; diff --git a/src/session.c b/src/session.c index 815d979..ccb6497 100644 --- a/src/session.c +++ b/src/session.c @@ -132,6 +132,10 @@ static int libssh2_banner_receive(LIBSSH2_SESSION *session) if (!banner_len) return 1; session->remote.banner = LIBSSH2_ALLOC(session, banner_len + 1); + if (!session->remote.banner) { + libssh2_error(session, LIBSSH2_ERROR_ALLOC, "Error allocating space for remote banner", 0); + return 1; + } memcpy(session->remote.banner, banner, banner_len); session->remote.banner[banner_len] = '\0'; _libssh2_debug(session, LIBSSH2_DBG_TRANS, "Received Banner: %s", session->remote.banner); @@ -233,15 +237,16 @@ LIBSSH2_API LIBSSH2_SESSION *libssh2_session_init_ex( local_realloc = my_realloc; session = local_alloc(sizeof(LIBSSH2_SESSION), abstract); - memset(session, 0, sizeof(LIBSSH2_SESSION)); - session->alloc = local_alloc; - session->free = local_free; - session->realloc = local_realloc; - session->abstract = abstract; - _libssh2_debug(session, LIBSSH2_DBG_TRANS, - "New session resource allocated"); - libssh2_crypto_init (); - + if (session) { + memset(session, 0, sizeof(LIBSSH2_SESSION)); + session->alloc = local_alloc; + session->free = local_free; + session->realloc = local_realloc; + session->abstract = abstract; + _libssh2_debug(session, LIBSSH2_DBG_TRANS, + "New session resource allocated"); + libssh2_crypto_init (); + } return session; } /* }}} */ diff --git a/src/userauth.c b/src/userauth.c index 7b1247c..5959c54 100644 --- a/src/userauth.c +++ b/src/userauth.c @@ -182,6 +182,7 @@ LIBSSH2_API int libssh2_userauth_password_ex(LIBSSH2_SESSION *session, const cha s = data = LIBSSH2_ALLOC(session, data_len); if (!data) { libssh2_error(session, LIBSSH2_ERROR_ALLOC, "Unable to allocate memory for userauth-password-change request", 0); + LIBSSH2_FREE(session, newpw); return -1; } @@ -206,6 +207,7 @@ LIBSSH2_API int libssh2_userauth_password_ex(LIBSSH2_SESSION *session, const cha if (libssh2_packet_write(session, data, data_len)) { libssh2_error(session, LIBSSH2_ERROR_SOCKET_SEND, "Unable to send userauth-password-change request", 0); LIBSSH2_FREE(session, data); + LIBSSH2_FREE(session, newpw); return -1; } LIBSSH2_FREE(session, data); @@ -373,6 +375,10 @@ LIBSSH2_API int libssh2_userauth_hostbased_fromfile_ex(LIBSSH2_SESSION *session, /* Preallocate space for an overall length, method name again, * and the signature, which won't be any larger than the size of the publickeydata itself */ s = packet = LIBSSH2_ALLOC(session, packet_len + 4 + (4 + method_len) + (4 + pubkeydata_len)); + if (!packet) { + LIBSSH2_FREE(session, method); + return -1; + } *(s++) = SSH_MSG_USERAUTH_REQUEST; libssh2_htonu32(s, username_len); s += 4; @@ -423,14 +429,18 @@ LIBSSH2_API int libssh2_userauth_hostbased_fromfile_ex(LIBSSH2_SESSION *session, privkeyobj->dtor(session, &abstract); } - if (sig_len > pubkeydata_len ) { + if (sig_len > pubkeydata_len) { + unsigned char *newpacket; /* Should *NEVER* happen, but...well.. better safe than sorry */ - packet = LIBSSH2_REALLOC(session, packet, packet_len + 4 + (4 + method_len) + (4 + sig_len)); /* PK sigblob */ - if (!packet) { + newpacket = LIBSSH2_REALLOC(session, packet, packet_len + 4 + (4 + method_len) + (4 + sig_len)); /* PK sigblob */ + if (!newpacket) { libssh2_error(session, LIBSSH2_ERROR_ALLOC, "Failed allocating additional space for userauth-hostbased packet", 0); + LIBSSH2_FREE(session, sig); + LIBSSH2_FREE(session, packet); LIBSSH2_FREE(session, method); return -1; } + packet = newpacket; } s = packet + packet_len; @@ -499,6 +509,11 @@ LIBSSH2_API int libssh2_userauth_publickey_fromfile_ex(LIBSSH2_SESSION *session, /* Preallocate space for an overall length, method name again, * and the signature, which won't be any larger than the size of the publickeydata itself */ s = packet = LIBSSH2_ALLOC(session, packet_len + 4 + (4 + method_len) + (4 + pubkeydata_len)); + if (!packet) { + LIBSSH2_FREE(session, method); + LIBSSH2_FREE(session, pubkeydata); + return -1; + } *(s++) = SSH_MSG_USERAUTH_REQUEST; libssh2_htonu32(s, username_len); s += 4; @@ -518,20 +533,19 @@ LIBSSH2_API int libssh2_userauth_publickey_fromfile_ex(LIBSSH2_SESSION *session, libssh2_htonu32(s, pubkeydata_len); s += 4; memcpy(s, pubkeydata, pubkeydata_len); s += pubkeydata_len; + LIBSSH2_FREE(session, pubkeydata); _libssh2_debug(session, LIBSSH2_DBG_AUTH, "Attempting publickey authentication"); if (libssh2_packet_write(session, packet, packet_len)) { libssh2_error(session, LIBSSH2_ERROR_SOCKET_SEND, "Unable to send userauth-publickey request", 0); LIBSSH2_FREE(session, packet); LIBSSH2_FREE(session, method); - LIBSSH2_FREE(session, pubkeydata); return -1; } if (libssh2_packet_requirev(session, reply_codes, &data, &data_len)) { LIBSSH2_FREE(session, packet); LIBSSH2_FREE(session, method); - LIBSSH2_FREE(session, pubkeydata); return -1; } @@ -541,7 +555,6 @@ LIBSSH2_API int libssh2_userauth_publickey_fromfile_ex(LIBSSH2_SESSION *session, LIBSSH2_FREE(session, data); LIBSSH2_FREE(session, packet); LIBSSH2_FREE(session, method); - LIBSSH2_FREE(session, pubkeydata); session->state |= LIBSSH2_STATE_AUTHENTICATED; return 0; } @@ -551,14 +564,12 @@ LIBSSH2_API int libssh2_userauth_publickey_fromfile_ex(LIBSSH2_SESSION *session, LIBSSH2_FREE(session, data); LIBSSH2_FREE(session, packet); LIBSSH2_FREE(session, method); - LIBSSH2_FREE(session, pubkeydata); libssh2_error(session, LIBSSH2_ERROR_PUBLICKEY_UNRECOGNIZED, "Username/PublicKey combination invalid", 0); return -1; } /* Semi-Success! */ LIBSSH2_FREE(session, data); - LIBSSH2_FREE(session, pubkeydata); if (libssh2_file_read_privatekey(session, &privkeyobj, &abstract, method, method_len, privatekey, passphrase)) { LIBSSH2_FREE(session, method); @@ -590,13 +601,17 @@ LIBSSH2_API int libssh2_userauth_publickey_fromfile_ex(LIBSSH2_SESSION *session, } if (sig_len > pubkeydata_len) { + unsigned char *newpacket; /* Should *NEVER* happen, but...well.. better safe than sorry */ - packet = LIBSSH2_REALLOC(session, packet, packet_len + 4 + (4 + method_len) + (4 + sig_len)); /* PK sigblob */ - if (!packet) { + newpacket = LIBSSH2_REALLOC(session, packet, packet_len + 4 + (4 + method_len) + (4 + sig_len)); /* PK sigblob */ + if (!newpacket) { libssh2_error(session, LIBSSH2_ERROR_ALLOC, "Failed allocating additional space for userauth-publickey packet", 0); + LIBSSH2_FREE(session, sig); + LIBSSH2_FREE(session, packet); LIBSSH2_FREE(session, method); return -1; } + packet = newpacket; } s = packet + packet_len;