From bb0548b3b00ca1b0b06e4d0ccf6cb794337eb192 Mon Sep 17 00:00:00 2001
From: Matt Johnston <matt@ucc.asn.au>
Date: Sun, 13 Jan 2008 03:55:59 +0000
Subject: [PATCH] Make a copy of passwd fields since getpwnam()'s retval isn't
 safe to keep

--HG--
extra : convert_revision : 295b11312e327fe6c4f33512674ea4a1a9790344
---
 auth.h            |  9 ++++--
 svr-agentfwd.c    |  8 ++---
 svr-auth.c        | 75 ++++++++++++++++++++++++++++++++++-------------
 svr-authpam.c     |  8 ++---
 svr-authpasswd.c  | 10 +++----
 svr-authpubkey.c  | 18 ++++++------
 svr-chansession.c | 36 +++++++++++++----------
 svr-session.c     |  6 ++--
 8 files changed, 106 insertions(+), 64 deletions(-)

diff --git a/auth.h b/auth.h
index 661265ac..b8e55bb5 100644
--- a/auth.h
+++ b/auth.h
@@ -91,9 +91,12 @@ struct AuthState {
 							   logged. */
 
 	/* These are only used for the server */
-	char *printableuser; /* stripped of control chars, used for logs etc */
-	struct passwd * pw;
-
+	uid_t pw_uid;
+	gid_t pw_gid;
+	char *pw_dir;
+	char *pw_shell;
+	char *pw_name;
+	char *pw_passwd;
 };
 
 struct SignKeyList;
diff --git a/svr-agentfwd.c b/svr-agentfwd.c
index 51271581..6946d1d4 100644
--- a/svr-agentfwd.c
+++ b/svr-agentfwd.c
@@ -150,8 +150,8 @@ void agentcleanup(struct ChanSess * chansess) {
 		 * for themselves */
 		uid = getuid();
 		gid = getgid();
-		if ((setegid(ses.authstate.pw->pw_gid)) < 0 ||
-			(seteuid(ses.authstate.pw->pw_uid)) < 0) {
+		if ((setegid(ses.authstate.pw_gid)) < 0 ||
+			(seteuid(ses.authstate.pw_uid)) < 0) {
 			dropbear_exit("failed to set euid");
 		}
 
@@ -213,8 +213,8 @@ static int bindagent(int fd, struct ChanSess * chansess) {
 	/* drop to user privs to make the dir/file */
 	uid = getuid();
 	gid = getgid();
-	if ((setegid(ses.authstate.pw->pw_gid)) < 0 ||
-		(seteuid(ses.authstate.pw->pw_uid)) < 0) {
+	if ((setegid(ses.authstate.pw_gid)) < 0 ||
+		(seteuid(ses.authstate.pw_uid)) < 0) {
 		dropbear_exit("failed to set euid");
 	}
 
diff --git a/svr-auth.c b/svr-auth.c
index ea31e796..7fb6568b 100644
--- a/svr-auth.c
+++ b/svr-auth.c
@@ -42,6 +42,10 @@ static void send_msg_userauth_banner();
 void svr_authinitialise() {
 
 	ses.authstate.failcount = 0;
+	ses.authstate.pw_name = NULL;
+	ses.authstate.pw_dir = NULL;
+	ses.authstate.pw_shell = NULL;
+	ses.authstate.pw_passwd = NULL;
 	authclear();
 	
 }
@@ -60,7 +64,19 @@ static void authclear() {
 		ses.authstate.authtypes |= AUTH_TYPE_PASSWORD;
 	}
 #endif
-
+	if (ses.authstate.pw_name) {
+		m_free(ses.authstate.pw_name);
+	}
+	if (ses.authstate.pw_shell) {
+		m_free(ses.authstate.pw_shell);
+	}
+	if (ses.authstate.pw_dir) {
+		m_free(ses.authstate.pw_dir);
+	}
+	if (ses.authstate.pw_passwd) {
+		m_free(ses.authstate.pw_passwd);
+	}
+	
 }
 
 /* Send a banner message if specified to the client. The client might
@@ -143,7 +159,7 @@ void recv_msg_userauth_request() {
 
 #ifdef ENABLE_SVR_PASSWORD_AUTH
 	if (!svr_opts.noauthpass &&
-			!(svr_opts.norootpass && ses.authstate.pw->pw_uid == 0) ) {
+			!(svr_opts.norootpass && ses.authstate.pw_uid == 0) ) {
 		/* user wants to try password auth */
 		if (methodlen == AUTH_METHOD_PASSWORD_LEN &&
 				strncmp(methodname, AUTH_METHOD_PASSWORD,
@@ -156,7 +172,7 @@ void recv_msg_userauth_request() {
 
 #ifdef ENABLE_SVR_PAM_AUTH
 	if (!svr_opts.noauthpass &&
-			!(svr_opts.norootpass && ses.authstate.pw->pw_uid == 0) ) {
+			!(svr_opts.norootpass && ses.authstate.pw_uid == 0) ) {
 		/* user wants to try password auth */
 		if (methodlen == AUTH_METHOD_PASSWORD_LEN &&
 				strncmp(methodname, AUTH_METHOD_PASSWORD,
@@ -187,6 +203,30 @@ out:
 	m_free(methodname);
 }
 
+static int fill_passwd(const char* username) {
+	struct passwd *pw = NULL;
+	if (ses.authstate.pw_name)
+		m_free(ses.authstate.pw_name);
+	if (ses.authstate.pw_dir)
+		m_free(ses.authstate.pw_dir);
+	if (ses.authstate.pw_shell)
+		m_free(ses.authstate.pw_shell);
+	if (ses.authstate.pw_passwd)
+		m_free(ses.authstate.pw_passwd);
+
+	pw = getpwnam(username);
+	if (!pw) {
+		return;
+	}
+	ses.authstate.pw_uid = pw->pw_uid;
+	ses.authstate.pw_gid = pw->pw_gid;
+	ses.authstate.pw_name = m_strdup(pw->pw_name);
+	ses.authstate.pw_dir = m_strdup(pw->pw_dir);
+	ses.authstate.pw_shell = m_strdup(pw->pw_shell);
+	ses.authstate.pw_passwd = m_strdup(pw->pw_passwd);
+}
+
+
 /* Check that the username exists, has a non-empty password, and has a valid
  * shell.
  * returns DROPBEAR_SUCCESS on valid username, DROPBEAR_FAILURE on failure */
@@ -194,7 +234,6 @@ static int checkusername(unsigned char *username, unsigned int userlen) {
 
 	char* listshell = NULL;
 	char* usershell = NULL;
-	
 	TRACE(("enter checkusername"))
 	if (userlen > MAX_USERNAME_LEN) {
 		return DROPBEAR_FAILURE;
@@ -210,13 +249,12 @@ static int checkusername(unsigned char *username, unsigned int userlen) {
 				m_free(ses.authstate.username);
 			}
 			authclear();
-			ses.authstate.pw = getpwnam((char*)username);
+			fill_passwd(username);
 			ses.authstate.username = m_strdup(username);
-			m_free(ses.authstate.printableuser);
 	}
 
 	/* check that user exists */
-	if (ses.authstate.pw == NULL) {
+	if (!ses.authstate.pw_name) {
 		TRACE(("leave checkusername: user '%s' doesn't exist", username))
 		dropbear_log(LOG_WARNING,
 				"login attempt for nonexistent user from %s",
@@ -225,11 +263,8 @@ static int checkusername(unsigned char *username, unsigned int userlen) {
 		return DROPBEAR_FAILURE;
 	}
 
-	/* We can set it once we know its a real user */
-	ses.authstate.printableuser = m_strdup(ses.authstate.pw->pw_name);
-
 	/* check for non-root if desired */
-	if (svr_opts.norootlogin && ses.authstate.pw->pw_uid == 0) {
+	if (svr_opts.norootlogin && ses.authstate.pw_uid == 0) {
 		TRACE(("leave checkusername: root login disabled"))
 		dropbear_log(LOG_WARNING, "root login rejected");
 		send_msg_userauth_failure(0, 1);
@@ -237,18 +272,18 @@ static int checkusername(unsigned char *username, unsigned int userlen) {
 	}
 
 	/* check for an empty password */
-	if (ses.authstate.pw->pw_passwd[0] == '\0') {
+	if (ses.authstate.pw_passwd[0] == '\0') {
 		TRACE(("leave checkusername: empty pword"))
 		dropbear_log(LOG_WARNING, "user '%s' has blank password, rejected",
-				ses.authstate.printableuser);
+				ses.authstate.pw_name);
 		send_msg_userauth_failure(0, 1);
 		return DROPBEAR_FAILURE;
 	}
 
-	TRACE(("shell is %s", ses.authstate.pw->pw_shell))
+	TRACE(("shell is %s", ses.authstate.pw_shell))
 
 	/* check that the shell is set */
-	usershell = ses.authstate.pw->pw_shell;
+	usershell = ses.authstate.pw_shell;
 	if (usershell[0] == '\0') {
 		/* empty shell in /etc/passwd means /bin/sh according to passwd(5) */
 		usershell = "/bin/sh";
@@ -269,7 +304,7 @@ static int checkusername(unsigned char *username, unsigned int userlen) {
 	endusershell();
 	TRACE(("no matching shell"))
 	dropbear_log(LOG_WARNING, "user '%s' has invalid shell, rejected",
-				ses.authstate.printableuser);
+				ses.authstate.pw_name);
 	send_msg_userauth_failure(0, 1);
 	return DROPBEAR_FAILURE;
 	
@@ -277,7 +312,7 @@ goodshell:
 	endusershell();
 	TRACE(("matching shell"))
 
-	TRACE(("uid = %d", ses.authstate.pw->pw_uid))
+	TRACE(("uid = %d", ses.authstate.pw_uid))
 	TRACE(("leave checkusername"))
 	return DROPBEAR_SUCCESS;
 
@@ -334,10 +369,10 @@ void send_msg_userauth_failure(int partial, int incrfail) {
 		/* XXX - send disconnect ? */
 		TRACE(("Max auth tries reached, exiting"))
 
-		if (ses.authstate.printableuser == NULL) {
+		if (ses.authstate.pw_name == NULL) {
 			userstr = "is invalid";
 		} else {
-			userstr = ses.authstate.printableuser;
+			userstr = ses.authstate.pw_name;
 		}
 		dropbear_exit("Max auth tries reached - user '%s' from %s",
 				userstr, svr_ses.addrstring);
@@ -360,7 +395,7 @@ void send_msg_userauth_success() {
 	ses.connect_time = 0;
 
 
-	if (ses.authstate.pw->pw_uid == 0) {
+	if (ses.authstate.pw_uid == 0) {
 		ses.allowprivport = 1;
 	}
 
diff --git a/svr-authpam.c b/svr-authpam.c
index ee7d6baf..8d6a6e76 100644
--- a/svr-authpam.c
+++ b/svr-authpam.c
@@ -195,7 +195,7 @@ void svr_auth_pam() {
 	/* used to pass data to the PAM conversation function - don't bother with
 	 * strdup() etc since these are touched only by our own conversation
 	 * function (above) which takes care of it */
-	userData.user = ses.authstate.printableuser;
+	userData.user = ses.authstate.pw_name;
 	userData.passwd = password;
 
 	/* Init pam */
@@ -221,7 +221,7 @@ void svr_auth_pam() {
 				rc, pam_strerror(pamHandlep, rc));
 		dropbear_log(LOG_WARNING,
 				"bad PAM password attempt for '%s' from %s",
-				ses.authstate.printableuser,
+				ses.authstate.pw_name,
 				svr_ses.addrstring);
 		send_msg_userauth_failure(0, 1);
 		goto cleanup;
@@ -232,7 +232,7 @@ void svr_auth_pam() {
 				rc, pam_strerror(pamHandlep, rc));
 		dropbear_log(LOG_WARNING,
 				"bad PAM password attempt for '%s' from %s",
-				ses.authstate.printableuser,
+				ses.authstate.pw_name,
 				svr_ses.addrstring);
 		send_msg_userauth_failure(0, 1);
 		goto cleanup;
@@ -240,7 +240,7 @@ void svr_auth_pam() {
 
 	/* successful authentication */
 	dropbear_log(LOG_NOTICE, "PAM password auth succeeded for '%s' from %s",
-			ses.authstate.printableuser,
+			ses.authstate.pw_name,
 			svr_ses.addrstring);
 	send_msg_userauth_success();
 
diff --git a/svr-authpasswd.c b/svr-authpasswd.c
index 5be1e2ad..53550a23 100644
--- a/svr-authpasswd.c
+++ b/svr-authpasswd.c
@@ -46,10 +46,10 @@ void svr_auth_password() {
 
 	unsigned int changepw;
 
-	passwdcrypt = ses.authstate.pw->pw_passwd;
+	passwdcrypt = ses.authstate.pw_passwd;
 #ifdef HAVE_SHADOW_H
 	/* get the shadow password if possible */
-	spasswd = getspnam(ses.authstate.printableuser);
+	spasswd = getspnam(ses.authstate.pw_name);
 	if (spasswd != NULL && spasswd->sp_pwdp != NULL) {
 		passwdcrypt = spasswd->sp_pwdp;
 	}
@@ -65,7 +65,7 @@ void svr_auth_password() {
 	 * in auth.c */
 	if (passwdcrypt[0] == '\0') {
 		dropbear_log(LOG_WARNING, "user '%s' has blank password, rejected",
-				ses.authstate.printableuser);
+				ses.authstate.pw_name);
 		send_msg_userauth_failure(0, 1);
 		return;
 	}
@@ -89,13 +89,13 @@ void svr_auth_password() {
 		/* successful authentication */
 		dropbear_log(LOG_NOTICE, 
 				"password auth succeeded for '%s' from %s",
-				ses.authstate.printableuser,
+				ses.authstate.pw_name,
 				svr_ses.addrstring);
 		send_msg_userauth_success();
 	} else {
 		dropbear_log(LOG_WARNING,
 				"bad password attempt for '%s' from %s",
-				ses.authstate.printableuser,
+				ses.authstate.pw_name,
 				svr_ses.addrstring);
 		send_msg_userauth_failure(0, 1);
 	}
diff --git a/svr-authpubkey.c b/svr-authpubkey.c
index d611c892..71477de1 100644
--- a/svr-authpubkey.c
+++ b/svr-authpubkey.c
@@ -105,12 +105,12 @@ void svr_auth_pubkey() {
 				signbuf->len) == DROPBEAR_SUCCESS) {
 		dropbear_log(LOG_NOTICE,
 				"pubkey auth succeeded for '%s' with key %s from %s",
-				ses.authstate.printableuser, fp, svr_ses.addrstring);
+				ses.authstate.pw_name, fp, svr_ses.addrstring);
 		send_msg_userauth_success();
 	} else {
 		dropbear_log(LOG_WARNING,
 				"pubkey auth bad signature for '%s' with key %s from %s",
-				ses.authstate.printableuser, fp, svr_ses.addrstring);
+				ses.authstate.pw_name, fp, svr_ses.addrstring);
 		send_msg_userauth_failure(0, 1);
 	}
 	m_free(fp);
@@ -166,7 +166,7 @@ static int checkpubkey(unsigned char* algo, unsigned int algolen,
 	if (have_algo(algo, algolen, sshhostkey) == DROPBEAR_FAILURE) {
 		dropbear_log(LOG_WARNING,
 				"pubkey auth attempt with unknown algo for '%s' from %s",
-				ses.authstate.printableuser, svr_ses.addrstring);
+				ses.authstate.pw_name, svr_ses.addrstring);
 		goto out;
 	}
 
@@ -178,12 +178,12 @@ static int checkpubkey(unsigned char* algo, unsigned int algolen,
 
 	/* we don't need to check pw and pw_dir for validity, since
 	 * its been done in checkpubkeyperms. */
-	len = strlen(ses.authstate.pw->pw_dir);
+	len = strlen(ses.authstate.pw_dir);
 	/* allocate max required pathname storage,
 	 * = path + "/.ssh/authorized_keys" + '\0' = pathlen + 22 */
 	filename = m_malloc(len + 22);
 	snprintf(filename, len + 22, "%s/.ssh/authorized_keys", 
-				ses.authstate.pw->pw_dir);
+				ses.authstate.pw_dir);
 
 	/* open the file */
 	authfile = fopen(filename, "r");
@@ -266,18 +266,18 @@ static int checkpubkeyperms() {
 
 	TRACE(("enter checkpubkeyperms"))
 
-	if (ses.authstate.pw->pw_dir == NULL) {
+	if (ses.authstate.pw_dir == NULL) {
 		goto out;
 	}
 
-	if ((len = strlen(ses.authstate.pw->pw_dir)) == 0) {
+	if ((len = strlen(ses.authstate.pw_dir)) == 0) {
 		goto out;
 	}
 
 	/* allocate max required pathname storage,
 	 * = path + "/.ssh/authorized_keys" + '\0' = pathlen + 22 */
 	filename = m_malloc(len + 22);
-	strncpy(filename, ses.authstate.pw->pw_dir, len+1);
+	strncpy(filename, ses.authstate.pw_dir, len+1);
 
 	/* check ~ */
 	if (checkfileperm(filename) != DROPBEAR_SUCCESS) {
@@ -320,7 +320,7 @@ static int checkfileperm(char * filename) {
 		return DROPBEAR_FAILURE;
 	}
 	/* check ownership - user or root only*/
-	if (filestat.st_uid != ses.authstate.pw->pw_uid
+	if (filestat.st_uid != ses.authstate.pw_uid
 			&& filestat.st_uid != 0) {
 		badperm = 1;
 		TRACE(("wrong ownership"))
diff --git a/svr-chansession.c b/svr-chansession.c
index 619a4510..ddb51b03 100644
--- a/svr-chansession.c
+++ b/svr-chansession.c
@@ -524,6 +524,7 @@ static int sessionpty(struct ChanSess * chansess) {
 
 	unsigned int termlen;
 	unsigned char namebuf[65];
+	struct passwd * pw = NULL;
 
 	TRACE(("enter sessionpty"))
 	chansess->term = buf_getstring(ses.payload, &termlen);
@@ -547,7 +548,10 @@ static int sessionpty(struct ChanSess * chansess) {
 		dropbear_exit("out of memory"); /* TODO disconnect */
 	}
 
-	pty_setowner(ses.authstate.pw, chansess->tty);
+	pw = getpwnam(ses.authstate.pw_name);
+	if (!pw)
+		dropbear_exit("getpwnam failed after succeeding previously");
+	pty_setowner(pw, chansess->tty);
 
 	/* Set up the rows/col counts */
 	sessionwinchange(chansess);
@@ -604,10 +608,10 @@ static int sessioncommand(struct Channel *channel, struct ChanSess *chansess,
 #ifdef LOG_COMMANDS
 	if (chansess->cmd) {
 		dropbear_log(LOG_INFO, "user %s executing '%s'", 
-						ses.authstate.printableuser, chansess->cmd);
+						ses.authstate.pw_name, chansess->cmd);
 	} else {
 		dropbear_log(LOG_INFO, "user %s executing login shell", 
-						ses.authstate.printableuser);
+						ses.authstate.pw_name);
 	}
 #endif
 
@@ -795,10 +799,10 @@ static int ptycommand(struct Channel *channel, struct ChanSess *chansess) {
 			/* don't show the motd if ~/.hushlogin exists */
 
 			/* 11 == strlen("/hushlogin\0") */
-			len = strlen(ses.authstate.pw->pw_dir) + 11; 
+			len = strlen(ses.authstate.pw_dir) + 11; 
 
 			hushpath = m_malloc(len);
-			snprintf(hushpath, len, "%s/hushlogin", ses.authstate.pw->pw_dir);
+			snprintf(hushpath, len, "%s/hushlogin", ses.authstate.pw_dir);
 
 			if (stat(hushpath, &sb) < 0) {
 				/* more than a screenful is stupid IMHO */
@@ -908,12 +912,12 @@ static void execchild(struct ChanSess *chansess) {
 	/* We can only change uid/gid as root ... */
 	if (getuid() == 0) {
 
-		if ((setgid(ses.authstate.pw->pw_gid) < 0) ||
-			(initgroups(ses.authstate.pw->pw_name, 
-						ses.authstate.pw->pw_gid) < 0)) {
+		if ((setgid(ses.authstate.pw_gid) < 0) ||
+			(initgroups(ses.authstate.pw_name, 
+						ses.authstate.pw_gid) < 0)) {
 			dropbear_exit("error changing user group");
 		}
-		if (setuid(ses.authstate.pw->pw_uid) < 0) {
+		if (setuid(ses.authstate.pw_uid) < 0) {
 			dropbear_exit("error changing user");
 		}
 	} else {
@@ -924,29 +928,29 @@ static void execchild(struct ChanSess *chansess) {
 		 * usernames with the same uid, but differing groups, then the
 		 * differing groups won't be set (as with initgroups()). The solution
 		 * is for the sysadmin not to give out the UID twice */
-		if (getuid() != ses.authstate.pw->pw_uid) {
+		if (getuid() != ses.authstate.pw_uid) {
 			dropbear_exit("couldn't	change user as non-root");
 		}
 	}
 
 	/* an empty shell should be interpreted as "/bin/sh" */
-	if (ses.authstate.pw->pw_shell[0] == '\0') {
+	if (ses.authstate.pw_shell[0] == '\0') {
 		usershell = "/bin/sh";
 	} else {
-		usershell = ses.authstate.pw->pw_shell;
+		usershell = ses.authstate.pw_shell;
 	}
 
 	/* set env vars */
-	addnewvar("USER", ses.authstate.pw->pw_name);
-	addnewvar("LOGNAME", ses.authstate.pw->pw_name);
-	addnewvar("HOME", ses.authstate.pw->pw_dir);
+	addnewvar("USER", ses.authstate.pw_name);
+	addnewvar("LOGNAME", ses.authstate.pw_name);
+	addnewvar("HOME", ses.authstate.pw_dir);
 	addnewvar("SHELL", usershell);
 	if (chansess->term != NULL) {
 		addnewvar("TERM", chansess->term);
 	}
 
 	/* change directory */
-	if (chdir(ses.authstate.pw->pw_dir) < 0) {
+	if (chdir(ses.authstate.pw_dir) < 0) {
 		dropbear_exit("error changing directory");
 	}
 
diff --git a/svr-session.c b/svr-session.c
index 18fab6bc..5a8364a2 100644
--- a/svr-session.c
+++ b/svr-session.c
@@ -130,12 +130,12 @@ void svr_dropbear_exit(int exitcode, const char* format, va_list param) {
 		/* user has authenticated */
 		snprintf(fmtbuf, sizeof(fmtbuf),
 				"exit after auth (%s): %s", 
-				ses.authstate.printableuser, format);
-	} else if (ses.authstate.printableuser) {
+				ses.authstate.pw_name, format);
+	} else if (ses.authstate.pw_name) {
 		/* we have a potential user */
 		snprintf(fmtbuf, sizeof(fmtbuf), 
 				"exit before auth (user '%s', %d fails): %s",
-				ses.authstate.printableuser, ses.authstate.failcount, format);
+				ses.authstate.pw_name, ses.authstate.failcount, format);
 	} else {
 		/* before userauth */
 		snprintf(fmtbuf, sizeof(fmtbuf), 
-- 
GitLab