openssh unix dev - May 2006 - [PATCH 10/12 bugfix: openssh-4.3p2: memory leak

The variable local_user was allocated by xstrdup and is not freed or
pointed to in this branch.  This patch adds the xfree.  This entire set
of patches passed the regression tests on my system. Bug found by
Coverity.
Signed-off-by: Kylene Hall <kjhall at us.ibm.com>
---
sshconnect.c |    1 +
1 files changed, 1 insertion(+)

diff -uprN openssh-4.3p2/sshconnect.c openssh-4.3p2-kylie/sshconnect.c
--- openssh-4.3p2/sshconnect.c	2005-12-13 02:29:03.000000000 -0600
+++ openssh-4.3p2-kylie/sshconnect.c	2006-05-04 10:07:57.000000000 -0500
@@ -937,6 +937,7 @@ ssh_login(Sensitive *sensitive, const ch
 	} else {
 		ssh_kex(host, hostaddr);
 		ssh_userauth1(local_user, server_user, host, sensitive);
+		xfree(local_user);
 	}
 }

Kylene,

Why is it that the "local_user" is being freed only in case of SSH1
and
not in case of SSH2?

Thanks,
Srinath.

-----Original Message-----
From: openssh-unix-dev-bounces+srinath_balaraman=mentor.com at mindrot.org
[mailto:openssh-unix-dev-bounces+srinath_balaraman=mentor.com at mindrot.or
g] On Behalf Of Kylene Jo Hall
Sent: Monday, May 15, 2006 3:19 PM
To: openssh-devel
Subject: [PATCH 10/12 bugfix: openssh-4.3p2: memory leak

The variable local_user was allocated by xstrdup and is not freed or
pointed to in this branch.  This patch adds the xfree.  This entire set
of patches passed the regression tests on my system. Bug found by
Coverity.
Signed-off-by: Kylene Hall <kjhall at us.ibm.com>
---
sshconnect.c |    1 +
1 files changed, 1 insertion(+)

diff -uprN openssh-4.3p2/sshconnect.c openssh-4.3p2-kylie/sshconnect.c
--- openssh-4.3p2/sshconnect.c	2005-12-13 02:29:03.000000000 -0600
+++ openssh-4.3p2-kylie/sshconnect.c	2006-05-04 10:07:57.000000000
-0500
@@ -937,6 +937,7 @@ ssh_login(Sensitive *sensitive, const ch
 	} else {
 		ssh_kex(host, hostaddr);
 		ssh_userauth1(local_user, server_user, host, sensitive);
+		xfree(local_user);
 	}
 }
 


_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev at mindrot.org
http://www.mindrot.org/mailman/listinfo/openssh-unix-dev

It was because in the function ssh_userauth2 local_user is pointed to by
a variable (authctx.local_user).  However, upon closer examination that
variable is local to the ssh_userauth2 function and I think local_user
should be freed in both cases.  Here is an updated patch.

Signed-off-by: Kylene Hall <kjhall at us.ibm.com>
---
  sshconnect.c |    1 +
 1 files changed, 1 insertion(+)

--- openssh-4.3p2/sshconnect.c	2005-12-13 02:29:03.000000000 -0600
+++ openssh-4.3p2-kylie/sshconnect.c	2006-05-16 09:39:58.495647952 -0500
@@ -938,6 +938,7 @@ ssh_login(Sensitive *sensitive, const ch
 		ssh_kex(host, hostaddr);
 		ssh_userauth1(local_user, server_user, host, sensitive);
 	}
+	xfree(local_user);
 }
 
 void


On Tue, 2006-05-16 at 09:13 -0500, Balaraman, Srinath
wrote:
> Kylene,
> 
> Why is it that the "local_user" is being freed only in case of
SSH1 and
> not in case of SSH2?
> 
> Thanks,
> Srinath.
> 
> -----Original Message-----
> From: openssh-unix-dev-bounces+srinath_balaraman=mentor.com at mindrot.org
> [mailto:openssh-unix-dev-bounces+srinath_balaraman=mentor.com at mindrot.or
> g] On Behalf Of Kylene Jo Hall
> Sent: Monday, May 15, 2006 3:19 PM
> To: openssh-devel
> Subject: [PATCH 10/12 bugfix: openssh-4.3p2: memory leak
> 
> The variable local_user was allocated by xstrdup and is not freed or
> pointed to in this branch.  This patch adds the xfree.  This entire set
> of patches passed the regression tests on my system. Bug found by
> Coverity.
> Signed-off-by: Kylene Hall <kjhall at us.ibm.com>
> ---
> sshconnect.c |    1 +
> 1 files changed, 1 insertion(+)
> 
> diff -uprN openssh-4.3p2/sshconnect.c openssh-4.3p2-kylie/sshconnect.c
> --- openssh-4.3p2/sshconnect.c	2005-12-13 02:29:03.000000000 -0600
> +++ openssh-4.3p2-kylie/sshconnect.c	2006-05-04 10:07:57.000000000
> -0500
> @@ -937,6 +937,7 @@ ssh_login(Sensitive *sensitive, const ch
>  	} else {
>  		ssh_kex(host, hostaddr);
>  		ssh_userauth1(local_user, server_user, host, sensitive);
> +		xfree(local_user);
>  	}
>  }
>  
> 
> 
> _______________________________________________
> openssh-unix-dev mailing list
> openssh-unix-dev at mindrot.org
> http://www.mindrot.org/mailman/listinfo/openssh-unix-dev