* [bug: login] login command with -f flag fails to switch user account. @ 2018-06-13 17:01 Takashi Yano 2018-06-17 6:22 ` Takashi Yano 0 siblings, 1 reply; 6+ messages in thread From: Takashi Yano @ 2018-06-13 17:01 UTC (permalink / raw) To: cygwin Hi, I have found the login command with -f flag fails to switch user account correctly. This causes a severe security problem. User can get console having cyg_server rights without password by following steps. Prepare: 0.1. Install rsh-sever and rsh with inetutils packages. 0.2. Set them up to work properly. Steps: 1.1. Make ~/.rhosts with line 'localhost' 1.2. Execute 'rlogin localhost'. Now you can get the cyg_server rights. This is caused by bug of login command. With the settings above, rlogind is executed as cyg_server account. If .rhosts is valid, rlogind executes login command with -f flag. This should switch the user account to the user specified. However, login command fails to switch the account. As a result, shell is executed as cyg_server account instead of specified user account. I looked into this problem, and found the bug is in login.c. The account information of targeted user is set to a pointer: struct passwd *pwd; by calling getpwnam(username). This pointer points the system static area. This area is overwritten with the account information of current user, i.e. cyg_server, by calling getpwuid(uid) in isROOT_UID(). getpwnam() and getpwuid() seems to share the same system area. login calls setuid(pwd->pw_uid) and setgid(pwd->pw_gid) to switch the account, however area pointed by pwd is already overwritten to the information of current user. As a result, the account switching is done to the same user account (cyg_server) though it should be done to the account specified. Above is the mechanism of this bug. -- Takashi Yano <takashi.yano@nifty.ne.jp> -- Problem reports: http://cygwin.com/problems.html FAQ: http://cygwin.com/faq/ Documentation: http://cygwin.com/docs.html Unsubscribe info: http://cygwin.com/ml/#unsubscribe-simple ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [bug: login] login command with -f flag fails to switch user account. 2018-06-13 17:01 [bug: login] login command with -f flag fails to switch user account Takashi Yano @ 2018-06-17 6:22 ` Takashi Yano 2018-06-17 22:31 ` Takashi Yano 0 siblings, 1 reply; 6+ messages in thread From: Takashi Yano @ 2018-06-17 6:22 UTC (permalink / raw) To: cygwin [-- Attachment #1: Type: text/plain, Size: 167 bytes --] Hi Corinna and Yaakov, I have created a patch against the git head to solve this problem. Could you please have a look? -- Takashi Yano <takashi.yano@nifty.ne.jp> [-- Attachment #2: 0001-Fixed-the-failure-in-the-case-of-trying-to-switch-us.patch --] [-- Type: application/octet-stream, Size: 2314 bytes --] From b070bb0f9ced001d3fee40a75a4882ff17d02428 Mon Sep 17 00:00:00 2001 From: Takashi Yano <takashi.yano@nifty.ne.jp> Date: Sat, 16 Jun 2018 21:59:30 +0900 Subject: [PATCH] Fixed the failure in the case of trying to switch user account with F flag. * login.c (main): Use getpwnam_r() instead of getpwnam() to prevent overwriting the area pointed by pwd by other calls of getpw* family functions. * login.c (isROOT_UID): Remove checking SeIncreaseQuotaPrivilege privilege because account cyg_server created by csih does not have this privilege. * winsec.c (getUserInfoForUID): Fix the first argument of the second NetUserGetInfo() call so that it can check user information locally if the machine is on a domain. --- login.c | 9 +++++---- winsec.c | 3 +-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/login.c b/login.c index 2ddfdcd..271d5e8 100644 --- a/login.c +++ b/login.c @@ -91,6 +91,8 @@ static void sleepexit (int eval); */ int timeout = 300; +struct passwd userpwd; +char pwbuf[16384]; struct passwd *pwd; int failures; char term[64], *hostname, *username, *tty; @@ -213,7 +215,7 @@ main (int argc, char **argv) } (void) strcpy (tbuf, username); #ifdef __CYGWIN__ - pwd = getpwnam (username); + getpwnam_r (username, &userpwd, pwbuf, sizeof(pwbuf), &pwd); #else if (pwd = getpwnam (username)) salt = pwd->pw_passwd; @@ -551,10 +553,9 @@ isROOT_UID (uid_t uid) { static const char *REQUIRED_PRIVS[] = { "SeAssignPrimaryTokenPrivilege", - "SeTcbPrivilege", - "SeIncreaseQuotaPrivilege" + "SeTcbPrivilege" }; - static const ULONG NUM_REQUIRED_PRIV = 3; + static const ULONG NUM_REQUIRED_PRIV = 2; OSVERSIONINFOEX osvi; struct passwd *pw; diff --git a/winsec.c b/winsec.c index 307a66b..13c5ecf 100644 --- a/winsec.c +++ b/winsec.c @@ -978,8 +978,7 @@ getUserInfoForUID (uid_t uid, /* if we had a domain, then try again locally. if we didn't have a domain, then the initial call WAS local */ if (NetUserGetInfo - (*uni_servername, (LPWSTR) & uni_name, level, - bufptr) != NERR_Success) + (NULL, (LPWSTR) & uni_name, level, bufptr) != NERR_Success) { syslog (LOG_ERR, "unable to obtain user info for %s [tried domain controller %s and localhost]\n", -- 2.17.0 [-- Attachment #3: Type: text/plain, Size: 219 bytes --] -- Problem reports: http://cygwin.com/problems.html FAQ: http://cygwin.com/faq/ Documentation: http://cygwin.com/docs.html Unsubscribe info: http://cygwin.com/ml/#unsubscribe-simple ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [bug: login] login command with -f flag fails to switch user account. 2018-06-17 6:22 ` Takashi Yano @ 2018-06-17 22:31 ` Takashi Yano 2018-06-17 22:31 ` Takashi Yano 0 siblings, 1 reply; 6+ messages in thread From: Takashi Yano @ 2018-06-17 22:31 UTC (permalink / raw) To: cygwin [-- Attachment #1: Type: text/plain, Size: 270 bytes --] Hi Corinna and Yaakov, On Sun, 17 Jun 2018 10:13:13 +0900 Takashi Yano wrote: > I have created a patch against the git head to solve this problem. > Could you please have a look? Fix typo and wording in commitment message. -- Takashi Yano <takashi.yano@nifty.ne.jp> [-- Attachment #2: 0001-Fix-the-issue-that-login-command-with-f-flag-fails-t.patch --] [-- Type: application/octet-stream, Size: 2416 bytes --] From be7acbe15e81af2f4c65878644fff593e5b1294c Mon Sep 17 00:00:00 2001 From: Takashi Yano <takashi.yano@nifty.ne.jp> Date: Sat, 16 Jun 2018 21:59:30 +0900 Subject: [PATCH] Fix the issue that login command with -f flag fails to switch user account. * login.c (main): Use getpwnam_r() instead of getpwnam() to prevent the area pointed by pwd being overwritten by other calls of getpw* family functions. * login.c (isROOT_UID): Remove checking SeIncreaseQuotaPrivilege privilege because the account cyg_server created by csih does not have this privilege. * winsec.c (getUserInfoForUID): Fix the first argument of the second NetUserGetInfo() call so that it can check user information locally if the machine is on a domain. Refer to the following post for detail. https://cygwin.com/ml/cygwin/2018-06/msg00146.html --- login.c | 9 +++++---- winsec.c | 3 +-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/login.c b/login.c index 2ddfdcd..271d5e8 100644 --- a/login.c +++ b/login.c @@ -91,6 +91,8 @@ static void sleepexit (int eval); */ int timeout = 300; +struct passwd userpwd; +char pwbuf[16384]; struct passwd *pwd; int failures; char term[64], *hostname, *username, *tty; @@ -213,7 +215,7 @@ main (int argc, char **argv) } (void) strcpy (tbuf, username); #ifdef __CYGWIN__ - pwd = getpwnam (username); + getpwnam_r (username, &userpwd, pwbuf, sizeof(pwbuf), &pwd); #else if (pwd = getpwnam (username)) salt = pwd->pw_passwd; @@ -551,10 +553,9 @@ isROOT_UID (uid_t uid) { static const char *REQUIRED_PRIVS[] = { "SeAssignPrimaryTokenPrivilege", - "SeTcbPrivilege", - "SeIncreaseQuotaPrivilege" + "SeTcbPrivilege" }; - static const ULONG NUM_REQUIRED_PRIV = 3; + static const ULONG NUM_REQUIRED_PRIV = 2; OSVERSIONINFOEX osvi; struct passwd *pw; diff --git a/winsec.c b/winsec.c index 307a66b..13c5ecf 100644 --- a/winsec.c +++ b/winsec.c @@ -978,8 +978,7 @@ getUserInfoForUID (uid_t uid, /* if we had a domain, then try again locally. if we didn't have a domain, then the initial call WAS local */ if (NetUserGetInfo - (*uni_servername, (LPWSTR) & uni_name, level, - bufptr) != NERR_Success) + (NULL, (LPWSTR) & uni_name, level, bufptr) != NERR_Success) { syslog (LOG_ERR, "unable to obtain user info for %s [tried domain controller %s and localhost]\n", -- 2.17.0 [-- Attachment #3: Type: text/plain, Size: 219 bytes --] -- Problem reports: http://cygwin.com/problems.html FAQ: http://cygwin.com/faq/ Documentation: http://cygwin.com/docs.html Unsubscribe info: http://cygwin.com/ml/#unsubscribe-simple ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [bug: login] login command with -f flag fails to switch user account. 2018-06-17 22:31 ` Takashi Yano @ 2018-06-17 22:31 ` Takashi Yano 2018-06-18 14:48 ` Corinna Vinschen 0 siblings, 1 reply; 6+ messages in thread From: Takashi Yano @ 2018-06-17 22:31 UTC (permalink / raw) To: cygwin [-- Attachment #1: Type: text/plain, Size: 194 bytes --] On Sun, 17 Jun 2018 11:14:33 +0900 Takashi Yano <takashi.yano@nifty.ne.jp> wrote: > Fix typo and wording in commitment message. Fixed again. Sorry. -- Takashi Yano <takashi.yano@nifty.ne.jp> [-- Attachment #2: 0001-Fix-the-issue-that-login-command-with-f-flag-fails-t.patch --] [-- Type: application/octet-stream, Size: 2426 bytes --] From b8669c90e017e0b6481cb1ebaa8e9be321fcc0aa Mon Sep 17 00:00:00 2001 From: Takashi Yano <takashi.yano@nifty.ne.jp> Date: Sat, 16 Jun 2018 21:59:30 +0900 Subject: [PATCH] Fix the issue that login command with -f flag fails to switch user account. * login.c (main): Use getpwnam_r() instead of getpwnam() to prevent the area pointed to by pwd from being overwritten by other calls of getpw*() family functions. * login.c (isROOT_UID): Remove checking SeIncreaseQuotaPrivilege privilege because the account cyg_server created by csih does not have this privilege. * winsec.c (getUserInfoForUID): Fix the first argument of the second NetUserGetInfo() call so that it can check user information locally if the machine is on a domain. Refer to the following post for detail. https://cygwin.com/ml/cygwin/2018-06/msg00146.html --- login.c | 9 +++++---- winsec.c | 3 +-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/login.c b/login.c index 2ddfdcd..271d5e8 100644 --- a/login.c +++ b/login.c @@ -91,6 +91,8 @@ static void sleepexit (int eval); */ int timeout = 300; +struct passwd userpwd; +char pwbuf[16384]; struct passwd *pwd; int failures; char term[64], *hostname, *username, *tty; @@ -213,7 +215,7 @@ main (int argc, char **argv) } (void) strcpy (tbuf, username); #ifdef __CYGWIN__ - pwd = getpwnam (username); + getpwnam_r (username, &userpwd, pwbuf, sizeof(pwbuf), &pwd); #else if (pwd = getpwnam (username)) salt = pwd->pw_passwd; @@ -551,10 +553,9 @@ isROOT_UID (uid_t uid) { static const char *REQUIRED_PRIVS[] = { "SeAssignPrimaryTokenPrivilege", - "SeTcbPrivilege", - "SeIncreaseQuotaPrivilege" + "SeTcbPrivilege" }; - static const ULONG NUM_REQUIRED_PRIV = 3; + static const ULONG NUM_REQUIRED_PRIV = 2; OSVERSIONINFOEX osvi; struct passwd *pw; diff --git a/winsec.c b/winsec.c index 307a66b..13c5ecf 100644 --- a/winsec.c +++ b/winsec.c @@ -978,8 +978,7 @@ getUserInfoForUID (uid_t uid, /* if we had a domain, then try again locally. if we didn't have a domain, then the initial call WAS local */ if (NetUserGetInfo - (*uni_servername, (LPWSTR) & uni_name, level, - bufptr) != NERR_Success) + (NULL, (LPWSTR) & uni_name, level, bufptr) != NERR_Success) { syslog (LOG_ERR, "unable to obtain user info for %s [tried domain controller %s and localhost]\n", -- 2.17.0 [-- Attachment #3: Type: text/plain, Size: 219 bytes --] -- Problem reports: http://cygwin.com/problems.html FAQ: http://cygwin.com/faq/ Documentation: http://cygwin.com/docs.html Unsubscribe info: http://cygwin.com/ml/#unsubscribe-simple ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [bug: login] login command with -f flag fails to switch user account. 2018-06-17 22:31 ` Takashi Yano @ 2018-06-18 14:48 ` Corinna Vinschen 2018-06-19 2:50 ` Yaakov Selkowitz 0 siblings, 1 reply; 6+ messages in thread From: Corinna Vinschen @ 2018-06-18 14:48 UTC (permalink / raw) To: cygwin; +Cc: Yaakov Selkowitz [-- Attachment #1: Type: text/plain, Size: 620 bytes --] On Jun 17 12:42, Takashi Yano wrote: > On Sun, 17 Jun 2018 11:14:33 +0900 > Takashi Yano <takashi.yano@nifty.ne.jp> wrote: > > Fix typo and wording in commitment message. > > Fixed again. Sorry. No worries. Thanks for the patch, I pushed it. Yaakov, would you mind to rebuild the login package? I'm not sure what to do with this meson stuff. I have a local build using the standard method, though. If that's ok I just upload it. Thanks, Corinna -- Corinna Vinschen Please, send mails regarding Cygwin to Cygwin Maintainer cygwin AT cygwin DOT com Red Hat [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [bug: login] login command with -f flag fails to switch user account. 2018-06-18 14:48 ` Corinna Vinschen @ 2018-06-19 2:50 ` Yaakov Selkowitz 0 siblings, 0 replies; 6+ messages in thread From: Yaakov Selkowitz @ 2018-06-19 2:50 UTC (permalink / raw) To: cygwin On 2018-06-18 07:17, Corinna Vinschen wrote: > On Jun 17 12:42, Takashi Yano wrote: >> On Sun, 17 Jun 2018 11:14:33 +0900 >> Takashi Yano <takashi.yano@nifty.ne.jp> wrote: >>> Fix typo and wording in commitment message. >> >> Fixed again. Sorry. > > No worries. Thanks for the patch, I pushed it. > > Yaakov, would you mind to rebuild the login package? I'm not sure what > to do with this meson stuff. I have a local build using the standard > method, though. If that's ok I just upload it. That should be fine, go ahead. -- Yaakov -- Problem reports: http://cygwin.com/problems.html FAQ: http://cygwin.com/faq/ Documentation: http://cygwin.com/docs.html Unsubscribe info: http://cygwin.com/ml/#unsubscribe-simple ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-06-18 14:48 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-06-13 17:01 [bug: login] login command with -f flag fails to switch user account Takashi Yano 2018-06-17 6:22 ` Takashi Yano 2018-06-17 22:31 ` Takashi Yano 2018-06-17 22:31 ` Takashi Yano 2018-06-18 14:48 ` Corinna Vinschen 2018-06-19 2:50 ` Yaakov Selkowitz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).