In sshconnect.c there are two global variables for server_version_string
client_version_string.
These are used just in a few functions and can easily be passed as
parameters.
Also, there is a strange construct, where their memory is allocated to
the global pointers, then copies of these pointers are assigned to the
kex structure. The kex_free finally frees them via cleanup of the kex
structure while the global pointers remain.
This can easily be rearranged so that is clearer who owns which memory
and who is thus responsible for freeing.
Also, the the ssh_login function leaks the memory allocated to the host
variable.
Also, there is another global variable (previous_host_key) which could
be removed by placing it into the ssh/active_state structure, but which,
for now, could at least be made module-static.
Proposal: ssh-connect allocates the initial memory and creates values
and passes these as parameters to the relevant functions. It passes
them also to ssh_kex2 which makes a true copy. ssh-connect frees its
memory and the kex system finally frees its copy.
Along the way, fix the host leak also and make previous_host_key static
because both changes are trivial.
The patch is attached. It's from the portable version but applies to
the original version with a few lines offset.
diff --git a/sshconnect.c b/sshconnect.c
index 6d81927..7736bc2 100644
--- a/sshconnect.c
+++ b/sshconnect.c
@@ -69,9 +69,7 @@
#include "ssherr.h"
#include "authfd.h"
-char *client_version_string = NULL;
-char *server_version_string = NULL;
-struct sshkey *previous_host_key = NULL;
+static struct sshkey *previous_host_key = NULL;
static int matching_host_key_dns = 0;
@@ -605,16 +603,16 @@ ssh_connect(struct ssh *ssh, const char *host, struct addrinfo *addrs,
}
static void
-send_client_banner(int connection_out, int minor1)
+send_client_banner(int connection_out, int minor1, char **client_version_stringp)
{
/* Send our own protocol version identification. */
- xasprintf(&client_version_string, "SSH-%d.%d-%.100s\r\n",
+ xasprintf(client_version_stringp, "SSH-%d.%d-%.100s\r\n",
PROTOCOL_MAJOR_2, PROTOCOL_MINOR_2, SSH_VERSION);
- if (atomicio(vwrite, connection_out, client_version_string,
- strlen(client_version_string)) != strlen(client_version_string))
+ if (atomicio(vwrite, connection_out, *client_version_stringp,
+ strlen(*client_version_stringp)) != strlen(*client_version_stringp))
fatal("write: %.100s", strerror(errno));
- chop(client_version_string);
- debug("Local version string %.100s", client_version_string);
+ chop(*client_version_stringp);
+ debug("Local version string %.100s", *client_version_stringp);
}
/*
@@ -622,7 +620,7 @@ send_client_banner(int connection_out, int minor1)
* identification string.
*/
void
-ssh_exchange_identification(int timeout_ms)
+ssh_exchange_identification(int timeout_ms, char **client_version_stringp, char **server_version_stringp)
{
char buf[256], remote_version[256]; /* must be same size! */
int remote_major, remote_minor, mismatch;
@@ -632,7 +630,7 @@ ssh_exchange_identification(int timeout_ms)
size_t len;
int rc;
- send_client_banner(connection_out, 0);
+ send_client_banner(connection_out, 0, client_version_stringp);
/* Read other side's version identification. */
for (n = 0;;) {
@@ -673,13 +671,13 @@ ssh_exchange_identification(int timeout_ms)
break;
debug("ssh_exchange_identification: %s", buf);
}
- server_version_string = xstrdup(buf);
+ *server_version_stringp = xstrdup(buf);
/*
* Check that the versions match. In future this might accept
* several versions and set appropriate flags to handle them.
*/
- if (sscanf(server_version_string, "SSH-%d.%d-%[^\n]\n",
+ if (sscanf(*server_version_stringp, "SSH-%d.%d-%[^\n]\n",
&remote_major, &remote_minor, remote_version) != 3)
fatal("Bad remote protocol version identification: '%.100s'", buf);
debug("Remote protocol version %d.%d, remote software version %.100s",
@@ -705,7 +703,7 @@ ssh_exchange_identification(int timeout_ms)
if ((datafellows & SSH_BUG_RSASIGMD5) != 0)
logit("Server version \"%.100s\" uses unsafe RSA signature "
"scheme; disabling use of RSA keys", remote_version);
- chop(server_version_string);
+ chop(*server_version_stringp);
}
/* defaults to 'no' */
@@ -1406,6 +1404,8 @@ ssh_login(Sensitive *sensitive, const char *orighost,
{
char *host;
char *server_user, *local_user;
+ char *client_version_string = NULL;
+ char *server_version_string = NULL;
local_user = xstrdup(pw->pw_name);
server_user = options.user ? options.user : local_user;
@@ -1415,7 +1415,7 @@ ssh_login(Sensitive *sensitive, const char *orighost,
lowercase(host);
/* Exchange protocol version identification strings with the server. */
- ssh_exchange_identification(timeout_ms);
+ ssh_exchange_identification(timeout_ms, &client_version_string, &server_version_string);
/* Put the connection into non-blocking mode. */
packet_set_nonblocking();
@@ -1423,9 +1423,12 @@ ssh_login(Sensitive *sensitive, const char *orighost,
/* key exchange */
/* authenticate user */
debug("Authenticating to %s:%d as '%s'", host, port, server_user);
- ssh_kex2(host, hostaddr, port);
+ ssh_kex2(host, hostaddr, port, client_version_string, server_version_string);
ssh_userauth2(local_user, server_user, host, sensitive);
+ free(client_version_string);
+ free(server_version_string);
free(local_user);
+ free(host);
}
void
diff --git a/sshconnect.h b/sshconnect.h
index 890d857..d6de9fe 100644
--- a/sshconnect.h
+++ b/sshconnect.h
@@ -40,7 +40,7 @@ void ssh_kill_proxy_command(void);
void ssh_login(Sensitive *, const char *, struct sockaddr *, u_short,
struct passwd *, int);
-void ssh_exchange_identification(int);
+void ssh_exchange_identification(int, char **, char **);
int verify_host_key(char *, struct sockaddr *, struct sshkey *);
@@ -48,7 +48,7 @@ void get_hostfile_hostname_ipaddr(char *, struct sockaddr *, u_short,
char **, char **);
void ssh_kex(char *, struct sockaddr *);
-void ssh_kex2(char *, struct sockaddr *, u_short);
+void ssh_kex2(char *, struct sockaddr *, u_short, const char *, const char *);
void ssh_userauth1(const char *, const char *, char *, Sensitive *);
void ssh_userauth2(const char *, const char *, char *, Sensitive *);
diff --git a/sshconnect2.c b/sshconnect2.c
index cc527e1..8a9b6c0 100644
--- a/sshconnect2.c
+++ b/sshconnect2.c
@@ -78,8 +78,6 @@
#endif
/* import */
-extern char *client_version_string;
-extern char *server_version_string;
extern Options options;
/*
@@ -155,7 +153,7 @@ order_hostkeyalgs(char *host, struct sockaddr *hostaddr, u_short port)
}
void
-ssh_kex2(char *host, struct sockaddr *hostaddr, u_short port)
+ssh_kex2(char *host, struct sockaddr *hostaddr, u_short port, const char *client_version_string, const char *server_version_string)
{
char *myproposal[PROPOSAL_MAX] = { KEX_CLIENT };
char *s, *all_key;
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@xxxxxxxxxxx
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
