From f048546c8787f2f415d7072e32a6eef03282342b Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Mon, 7 Jul 2025 08:23:15 +0200 Subject: [PATCH] libssh2: remove use of 'initialised' for cleanup It could previously cause a memory-leak when the cleanup was not performed because it was not set. Reported-by: albrechtd on github Fixes #17819 Closes #17837 --- lib/vssh/libssh2.c | 157 ++++++++++++++++++++++----------------------- lib/vssh/ssh.h | 3 +- 2 files changed, 79 insertions(+), 81 deletions(-) diff --git a/lib/vssh/libssh2.c b/lib/vssh/libssh2.c index a8532e0b7b..5748f671d1 100644 --- a/lib/vssh/libssh2.c +++ b/lib/vssh/libssh2.c @@ -3245,7 +3245,6 @@ static CURLcode ssh_setup_connection(struct Curl_easy *data, if(!sshc) return CURLE_OUT_OF_MEMORY; - sshc->initialised = TRUE; if(Curl_conn_meta_set(conn, CURL_META_SSH_CONN, sshc, myssh_conn_dtor)) return CURLE_OUT_OF_MEMORY; @@ -3589,101 +3588,99 @@ static CURLcode sshc_cleanup(struct ssh_conn *sshc, struct Curl_easy *data, { int rc; - if(sshc->initialised) { - if(sshc->kh) { - libssh2_knownhost_free(sshc->kh); - sshc->kh = NULL; - } - - if(sshc->ssh_agent) { - rc = libssh2_agent_disconnect(sshc->ssh_agent); - if(!block && (rc == LIBSSH2_ERROR_EAGAIN)) - return CURLE_AGAIN; + if(sshc->kh) { + libssh2_knownhost_free(sshc->kh); + sshc->kh = NULL; + } - if((rc < 0) && data) { - char *err_msg = NULL; - (void)libssh2_session_last_error(sshc->ssh_session, - &err_msg, NULL, 0); - infof(data, "Failed to disconnect from libssh2 agent: %d %s", - rc, err_msg); - } - libssh2_agent_free(sshc->ssh_agent); - sshc->ssh_agent = NULL; + if(sshc->ssh_agent) { + rc = libssh2_agent_disconnect(sshc->ssh_agent); + if(!block && (rc == LIBSSH2_ERROR_EAGAIN)) + return CURLE_AGAIN; - /* NB: there is no need to free identities, they are part of internal - agent stuff */ - sshc->sshagent_identity = NULL; - sshc->sshagent_prev_identity = NULL; + if((rc < 0) && data) { + char *err_msg = NULL; + (void)libssh2_session_last_error(sshc->ssh_session, + &err_msg, NULL, 0); + infof(data, "Failed to disconnect from libssh2 agent: %d %s", + rc, err_msg); } + libssh2_agent_free(sshc->ssh_agent); + sshc->ssh_agent = NULL; - if(sshc->sftp_handle) { - rc = libssh2_sftp_close(sshc->sftp_handle); - if(!block && (rc == LIBSSH2_ERROR_EAGAIN)) - return CURLE_AGAIN; + /* NB: there is no need to free identities, they are part of internal + agent stuff */ + sshc->sshagent_identity = NULL; + sshc->sshagent_prev_identity = NULL; + } - if((rc < 0) && data) { - char *err_msg = NULL; - (void)libssh2_session_last_error(sshc->ssh_session, &err_msg, - NULL, 0); - infof(data, "Failed to close libssh2 file: %d %s", rc, err_msg); - } - sshc->sftp_handle = NULL; + if(sshc->sftp_handle) { + rc = libssh2_sftp_close(sshc->sftp_handle); + if(!block && (rc == LIBSSH2_ERROR_EAGAIN)) + return CURLE_AGAIN; + + if((rc < 0) && data) { + char *err_msg = NULL; + (void)libssh2_session_last_error(sshc->ssh_session, &err_msg, + NULL, 0); + infof(data, "Failed to close libssh2 file: %d %s", rc, err_msg); } + sshc->sftp_handle = NULL; + } - if(sshc->ssh_channel) { - rc = libssh2_channel_free(sshc->ssh_channel); - if(!block && (rc == LIBSSH2_ERROR_EAGAIN)) - return CURLE_AGAIN; + if(sshc->ssh_channel) { + rc = libssh2_channel_free(sshc->ssh_channel); + if(!block && (rc == LIBSSH2_ERROR_EAGAIN)) + return CURLE_AGAIN; - if((rc < 0) && data) { - char *err_msg = NULL; - (void)libssh2_session_last_error(sshc->ssh_session, - &err_msg, NULL, 0); - infof(data, "Failed to free libssh2 scp subsystem: %d %s", - rc, err_msg); - } - sshc->ssh_channel = NULL; + if((rc < 0) && data) { + char *err_msg = NULL; + (void)libssh2_session_last_error(sshc->ssh_session, + &err_msg, NULL, 0); + infof(data, "Failed to free libssh2 scp subsystem: %d %s", + rc, err_msg); } + sshc->ssh_channel = NULL; + } - if(sshc->sftp_session) { - rc = libssh2_sftp_shutdown(sshc->sftp_session); - if(!block && (rc == LIBSSH2_ERROR_EAGAIN)) - return CURLE_AGAIN; + if(sshc->sftp_session) { + rc = libssh2_sftp_shutdown(sshc->sftp_session); + if(!block && (rc == LIBSSH2_ERROR_EAGAIN)) + return CURLE_AGAIN; - if((rc < 0) && data) - infof(data, "Failed to stop libssh2 sftp subsystem"); - sshc->sftp_session = NULL; - } + if((rc < 0) && data) + infof(data, "Failed to stop libssh2 sftp subsystem"); + sshc->sftp_session = NULL; + } - if(sshc->ssh_session) { - rc = libssh2_session_free(sshc->ssh_session); - if(!block && (rc == LIBSSH2_ERROR_EAGAIN)) - return CURLE_AGAIN; + if(sshc->ssh_session) { + rc = libssh2_session_free(sshc->ssh_session); + if(!block && (rc == LIBSSH2_ERROR_EAGAIN)) + return CURLE_AGAIN; - if((rc < 0) && data) { - char *err_msg = NULL; - (void)libssh2_session_last_error(sshc->ssh_session, - &err_msg, NULL, 0); - infof(data, "Failed to free libssh2 session: %d %s", rc, err_msg); - } - sshc->ssh_session = NULL; + if((rc < 0) && data) { + char *err_msg = NULL; + (void)libssh2_session_last_error(sshc->ssh_session, + &err_msg, NULL, 0); + infof(data, "Failed to free libssh2 session: %d %s", rc, err_msg); } + sshc->ssh_session = NULL; + } - /* worst-case scenario cleanup */ - DEBUGASSERT(sshc->ssh_session == NULL); - DEBUGASSERT(sshc->ssh_channel == NULL); - DEBUGASSERT(sshc->sftp_session == NULL); - DEBUGASSERT(sshc->sftp_handle == NULL); - DEBUGASSERT(sshc->kh == NULL); - DEBUGASSERT(sshc->ssh_agent == NULL); + /* worst-case scenario cleanup */ + DEBUGASSERT(sshc->ssh_session == NULL); + DEBUGASSERT(sshc->ssh_channel == NULL); + DEBUGASSERT(sshc->sftp_session == NULL); + DEBUGASSERT(sshc->sftp_handle == NULL); + DEBUGASSERT(sshc->kh == NULL); + DEBUGASSERT(sshc->ssh_agent == NULL); + + Curl_safefree(sshc->rsa_pub); + Curl_safefree(sshc->rsa); + Curl_safefree(sshc->quote_path1); + Curl_safefree(sshc->quote_path2); + Curl_safefree(sshc->homedir); - Curl_safefree(sshc->rsa_pub); - Curl_safefree(sshc->rsa); - Curl_safefree(sshc->quote_path1); - Curl_safefree(sshc->quote_path2); - Curl_safefree(sshc->homedir); - sshc->initialised = FALSE; - } return CURLE_OK; } diff --git a/lib/vssh/ssh.h b/lib/vssh/ssh.h index feee886562..4056c39b20 100644 --- a/lib/vssh/ssh.h +++ b/lib/vssh/ssh.h @@ -191,6 +191,7 @@ struct ssh_conn { const char *readdir_filename; /* points within readdir_attrs */ const char *readdir_longentry; char *readdir_tmp; + BIT(initialised); #elif defined(USE_LIBSSH2) LIBSSH2_SESSION *ssh_session; /* Secure Shell session */ LIBSSH2_CHANNEL *ssh_channel; /* Secure Shell channel handle */ @@ -214,8 +215,8 @@ struct ssh_conn { word32 handleSz; byte handle[WOLFSSH_MAX_HANDLE]; curl_off_t offset; -#endif /* USE_LIBSSH */ BIT(initialised); +#endif /* USE_LIBSSH */ BIT(authed); /* the connection has been authenticated fine */ BIT(acceptfail); /* used by the SFTP_QUOTE (continue if quote command fails) */ -- 2.47.2