[bug: login] login command with -f flag fails to switch user account.

[bug: login] login command with -f flag fails to switch user account.

[Takashi Yano]

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

Re: [bug: login] login command with -f flag fails to switch user account.

[Takashi Yano]

[-- 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

Re: [bug: login] login command with -f flag fails to switch user account.

[Takashi Yano]

[-- 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

Re: [bug: login] login command with -f flag fails to switch user account.

[Takashi Yano]

[-- 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

Re: [bug: login] login command with -f flag fails to switch user account.

[Corinna Vinschen]

[-- 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 --]

Re: [bug: login] login command with -f flag fails to switch user account.

[Yaakov Selkowitz]

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