Corinna Vinschen
2001-Dec-18 19:20 UTC
[PATCH]: Fix potential security hole in Cygwin version
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
2001-Dec-28 09:17 UTC
[PATCH]: Fix potential security hole in Cygwin version
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