Markus Schmidt
2019-Feb-05 10:44 UTC
[PATCH] client leaks ssh context, host_arg and packet.c leaks kex-structure.
After completing the session, the client leaks the ssh structure and the
host_arg.
Also, on cleanup, packet.c leaks the kex-structure (which is held inside
the ssh struct).
I've also moved host_arg from global to local variable, because it is
only passed to one function.
Patch is attached
Best regards
Markus Schmidt
-------------- next part --------------
diff --git a/packet.c b/packet.c
index ec03301..d140a13 100644
--- a/packet.c
+++ b/packet.c
@@ -588,7 +588,7 @@ ssh_packet_close_internal(struct ssh *ssh, int do_close)
struct session_state *state = ssh->state;
u_int mode;
- if (!state->initialized)
+ if (state!=NULL || !state->initialized)
return;
state->initialized = 0;
if (do_close) {
@@ -638,6 +638,10 @@ ssh_packet_close_internal(struct ssh *ssh, int do_close)
cipher_free(state->receive_context);
state->send_context = state->receive_context = NULL;
if (do_close) {
+ if (ssh->kex) {
+ kex_free(ssh->kex);
+ ssh->kex = NULL;
+ }
free(ssh->local_ipaddr);
ssh->local_ipaddr = NULL;
free(ssh->remote_ipaddr);
diff --git a/ssh.c b/ssh.c
index 91e7c35..a040204 100644
--- a/ssh.c
+++ b/ssh.c
@@ -169,7 +169,7 @@ char *host;
/* Various strings used to to percent_expand() arguments */
static char thishost[NI_MAXHOST], shorthost[NI_MAXHOST], portstr[NI_MAXSERV];
-static char uidstr[32], *host_arg, *conn_hash_hex;
+static char uidstr[32], *conn_hash_hex;
/* socket address the host resolves to */
struct sockaddr_storage hostaddr;
@@ -207,7 +207,7 @@ usage(void)
exit(255);
}
-static int ssh_session2(struct ssh *, struct passwd *);
+static int ssh_session2(struct ssh *, struct passwd *, char *host_arg);
static void load_public_identity_files(struct passwd *);
static void main_sigchld_handler(int);
@@ -584,7 +584,7 @@ main(int ac, char **av)
struct ssh *ssh = NULL;
int i, r, opt, exit_status, use_syslog, direct, timeout_ms;
int was_addr, config_test = 0, opt_terminated = 0, want_final_pass = 0;
- char *p, *cp, *line, *argv0, buf[PATH_MAX], *logfile;
+ char *p, *cp, *line, *argv0, buf[PATH_MAX], *logfile, *host_arg;
char cname[NI_MAXHOST];
struct stat st;
struct passwd *pw;
@@ -1523,8 +1523,12 @@ main(int ac, char **av)
}
skip_connect:
- exit_status = ssh_session2(ssh, pw);
+ exit_status = ssh_session2(ssh, pw, host_arg);
ssh_packet_close(ssh);
+ free(ssh);
+ ssh = NULL;
+ free(host_arg);
+ host_arg = NULL;
if (options.control_path != NULL && muxserver_sock != -1)
unlink(options.control_path);
@@ -1873,7 +1877,7 @@ ssh_session2_open(struct ssh *ssh)
}
static int
-ssh_session2(struct ssh *ssh, struct passwd *pw)
+ssh_session2(struct ssh *ssh, struct passwd *pw, char *host_arg)
{
int r, devnull, id = -1;
char *cp, *tun_fwd_ifname = NULL;
Markus Schmidt
2019-Feb-05 14:11 UTC
[PATCH] v2 of client leaks ssh context, host_arg and packet.c leaks kex-structure.
The patch I sent with the earlier mail for the problem described below had a hunk (packet.c#590) that should not have been there. I also filed the correct patch to bugzila. https://bugzilla.mindrot.org/show_bug.cgi?id=2962 Sorry for the bad patch. Best regards Markus Schmidt On 02.05.19 11:44 , Markus Schmidt wrote:> After completing the session, the client leaks the ssh structure and the > host_arg. > > Also, on cleanup, packet.c leaks the kex-structure (which is held inside > the ssh struct). > > I've also moved host_arg from global to local variable, because it is > only passed to one function. > > Patch is attached-------------- next part -------------- diff --git a/packet.c b/packet.c index ec03301..a330916 100644 --- a/packet.c +++ b/packet.c @@ -638,6 +638,10 @@ ssh_packet_close_internal(struct ssh *ssh, int do_close) cipher_free(state->receive_context); state->send_context = state->receive_context = NULL; if (do_close) { + if (ssh->kex) { + kex_free(ssh->kex); + ssh->kex = NULL; + } free(ssh->local_ipaddr); ssh->local_ipaddr = NULL; free(ssh->remote_ipaddr); diff --git a/ssh.c b/ssh.c index 91e7c35..a040204 100644 --- a/ssh.c +++ b/ssh.c @@ -169,7 +169,7 @@ char *host; /* Various strings used to to percent_expand() arguments */ static char thishost[NI_MAXHOST], shorthost[NI_MAXHOST], portstr[NI_MAXSERV]; -static char uidstr[32], *host_arg, *conn_hash_hex; +static char uidstr[32], *conn_hash_hex; /* socket address the host resolves to */ struct sockaddr_storage hostaddr; @@ -207,7 +207,7 @@ usage(void) exit(255); } -static int ssh_session2(struct ssh *, struct passwd *); +static int ssh_session2(struct ssh *, struct passwd *, char *host_arg); static void load_public_identity_files(struct passwd *); static void main_sigchld_handler(int); @@ -584,7 +584,7 @@ main(int ac, char **av) struct ssh *ssh = NULL; int i, r, opt, exit_status, use_syslog, direct, timeout_ms; int was_addr, config_test = 0, opt_terminated = 0, want_final_pass = 0; - char *p, *cp, *line, *argv0, buf[PATH_MAX], *logfile; + char *p, *cp, *line, *argv0, buf[PATH_MAX], *logfile, *host_arg; char cname[NI_MAXHOST]; struct stat st; struct passwd *pw; @@ -1523,8 +1523,12 @@ main(int ac, char **av) } skip_connect: - exit_status = ssh_session2(ssh, pw); + exit_status = ssh_session2(ssh, pw, host_arg); ssh_packet_close(ssh); + free(ssh); + ssh = NULL; + free(host_arg); + host_arg = NULL; if (options.control_path != NULL && muxserver_sock != -1) unlink(options.control_path); @@ -1873,7 +1877,7 @@ ssh_session2_open(struct ssh *ssh) } static int -ssh_session2(struct ssh *ssh, struct passwd *pw) +ssh_session2(struct ssh *ssh, struct passwd *pw, char *host_arg) { int r, devnull, id = -1; char *cp, *tun_fwd_ifname = NULL; diff --git a/sshconnect2.c b/sshconnect2.c index 8ce309b..058a8c4 100644 --- a/sshconnect2.c +++ b/sshconnect2.c @@ -313,7 +313,6 @@ int userauth_kbdint(struct ssh *); int userauth_hostbased(struct ssh *); #ifdef GSSAPI -<<<<<<< HEAD int userauth_gssapi(struct ssh *); void userauth_gssapi_cleanup(struct ssh *); static int input_gssapi_response(int type, u_int32_t, struct ssh *);