[PATCH]: Fix potential security hole in Cygwin version

[Corinna Vinschen]

Hi,

could anybody please look into these Cygwin specific changes and either
apply them or tell me why they shouldn't get applied?

This patch

"[PATCH] Fix potential security hole in Cygwin version"

and the following two patches

"[PATCH]: Fix environment variable size restriction in Cygwin version"
"[PATCH]: Fix typo in contrib/cygwin/README"

are already part of OpenSSH-3.0.2p1-3 in the Cygwin net distribution but
they all belong into the main sources.

Thanks,
Corinna

On Tue, Dec 18, 2001 at 08:20:09PM +0100, Corinna Vinschen wrote:
> Hi,
> 
> the following patch fixes a potential security hole in the Cygwin
> version of sshd.
> 
> If you're logging in to a Cygwin sshd with version 2 protocol using an
> arbitrary user name which is not in /etc/passwd, the forked sshd which
> is handling this connection crashes with a segmentation violation.  The
> client side encounters an immediate disconnect ("Connection reset by   
> peer").  This could be used by a malicious remote client to enumerate
> the user names on the Cygwin server machine.
> 
> The cause is that the Cygwin specific function check_nt_auth() is called
> in auth1.c and auth2.c with implicitly dereferencing the pointer to struct
> passwd to get the pw_uid member as parameter.  This struct passwd pointer
> can be NULL if the user isn't found in /etc/passwd.  Other similar funcs
> as auth_pam_password() are called getting the structy passwd pointer
> itself as parameter, testing it for NULL inside of the function.
> 
> Changing the check_nt_auth() to behave that way and changing auth1.c and
> auth2.c accordingly solves that problem.
> 
> Patch follows.
> 
> Thanks,
> Corinna
> 
> Index: auth1.c
> ===================================================================
> RCS file: /cvs/openssh_cvs/auth1.c,v
> retrieving revision 1.46
> diff -u -p -r1.46 auth1.c
> --- auth1.c	6 Dec 2001 17:55:26 -0000	1.46
> +++ auth1.c	18 Dec 2001 19:07:12 -0000
> @@ -313,9 +313,9 @@ do_authloop(Authctxt *authctxt)
>  
>  #ifdef HAVE_CYGWIN
>  		if (authenticated &&
> -		    !check_nt_auth(type == SSH_CMSG_AUTH_PASSWORD,pw->pw_uid)) {
> +		    !check_nt_auth(type == SSH_CMSG_AUTH_PASSWORD, pw)) {
>  			packet_disconnect("Authentication rejected for uid %d.",
> -			(int)pw->pw_uid);
> +			pw ? (int)pw->pw_uid : -1);
>  			authenticated = 0;
>  		}
>  #else
> Index: auth2.c
> ===================================================================
> RCS file: /cvs/openssh_cvs/auth2.c,v
> retrieving revision 1.78
> diff -u -p -r1.78 auth2.c
> --- auth2.c	6 Dec 2001 17:55:26 -0000	1.78
> +++ auth2.c	18 Dec 2001 19:07:13 -0000
> @@ -341,7 +341,7 @@ userauth_none(Authctxt *authctxt)
>  		return(0);
>  
>  #ifdef HAVE_CYGWIN
> -	if (check_nt_auth(1, authctxt->pw->pw_uid) == 0)
> +	if (check_nt_auth(1, authctxt->pw) == 0)
>  		return(0);
>  #endif
>  #ifdef USE_PAM
> @@ -367,7 +367,7 @@ userauth_passwd(Authctxt *authctxt)
>  	packet_done();
>  	if (authctxt->valid &&
>  #ifdef HAVE_CYGWIN
> -		check_nt_auth(1, authctxt->pw->pw_uid) &&
> +	    check_nt_auth(1, authctxt->pw) &&
>  #endif
>  #ifdef USE_PAM
>  	    auth_pam_password(authctxt->pw, password) == 1)
> @@ -404,7 +404,7 @@ userauth_kbdint(Authctxt *authctxt)
>  	xfree(devs);
>  	xfree(lang);
>  #ifdef HAVE_CYGWIN
> -	if (check_nt_auth(0, authctxt->pw->pw_uid) == 0)
> +	if (check_nt_auth(0, authctxt->pw) == 0)
>  		return(0);
>  #endif
>  	return authenticated;
> @@ -510,7 +510,7 @@ userauth_pubkey(Authctxt *authctxt)
>  	xfree(pkalg);
>  	xfree(pkblob);
>  #ifdef HAVE_CYGWIN
> -	if (check_nt_auth(0, authctxt->pw->pw_uid) == 0)
> +	if (check_nt_auth(0, authctxt->pw) == 0)
>  		return(0);
>  #endif
>  	return authenticated;
> Index: openbsd-compat/bsd-cygwin_util.c
> ===================================================================
> RCS file: /cvs/openssh_cvs/openbsd-compat/bsd-cygwin_util.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 bsd-cygwin_util.c
> --- openbsd-compat/bsd-cygwin_util.c	27 Nov 2001 01:19:44 -0000	1.6
> +++ openbsd-compat/bsd-cygwin_util.c	18 Dec 2001 19:07:14 -0000
> @@ -58,7 +58,7 @@ int binary_pipe(int fd[2])
>  	return ret;
>  }
>  
> -int check_nt_auth(int pwd_authenticated, uid_t uid)
> +int check_nt_auth(int pwd_authenticated, struct passwd *pw)
>  {
>  	/*
>  	* The only authentication which is able to change the user
> @@ -73,6 +73,8 @@ int check_nt_auth(int pwd_authenticated,
>  	*/
>  	static int has_create_token = -1;
>  
> +	if (pw == NULL)
> +		return 0;
>  	if (is_winnt) {
>  		if (has_create_token < 0) {
>  			struct utsname uts;
> @@ -90,7 +92,7 @@ int check_nt_auth(int pwd_authenticated,
>  			}
>  		}
>  		if (has_create_token < 1 &&
> -		    !pwd_authenticated && geteuid() != uid)
> +		    !pwd_authenticated && geteuid() != pw->pw_uid)
>  			return 0;
>  	}
>  	return 1;
> Index: openbsd-compat/bsd-cygwin_util.h
> ===================================================================
> RCS file: /cvs/openssh_cvs/openbsd-compat/bsd-cygwin_util.h,v
> retrieving revision 1.5
> diff -u -p -r1.5 bsd-cygwin_util.h
> --- openbsd-compat/bsd-cygwin_util.h	27 Nov 2001 01:19:44 -0000	1.5
> +++ openbsd-compat/bsd-cygwin_util.h	18 Dec 2001 19:07:14 -0000
> @@ -24,7 +24,7 @@
>  
>  int binary_open(const char *filename, int flags, ...);
>  int binary_pipe(int fd[2]);
> -int check_nt_auth(int pwd_authenticated, uid_t uid);
> +int check_nt_auth(int pwd_authenticated, struct passwd *pw);
>  int check_ntsec(const char *filename);
>  void register_9x_service(void);
>  
> -- 
> Corinna Vinschen
> Cygwin Developer
> Red Hat, Inc.
> mailto:vinschen at redhat.com

-- 
Corinna Vinschen
Cygwin Developer
Red Hat, Inc.
mailto:vinschen at redhat.com