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

diff --git a/lib/vssh/libssh2.c b/lib/vssh/libssh2.c
index a8532e0b7b95869ff1b904c452371e1500b4fc69..5748f671d103954f2f01b9818e099a3c5395608f 100644 (file)
--- 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 feee886562e6ed77fe0592508def5472054da513..4056c39b2002d110e224dccdb6caf8f833b845d6 100644 (file)
--- 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) */