X-Git-Url: https://granicus.if.org/sourcecode?a=blobdiff_plain;f=src%2Fsu.c;h=f6d55c10990376549c4a0e8373c2824eaedd52b2;hb=a845c67c601cd08a01d6f120faae26bbc9e5c72c;hp=a48e5296f7f11a74c04917a944119db70b7d0138;hpb=b2120265fd5034a3974363c1ccafb6bbaf196c47;p=shadow

diff --git a/src/su.c b/src/su.c
index a48e5296..f6d55c10 100644
--- a/src/su.c
+++ b/src/su.c
@@ -1,5 +1,8 @@
 /*
- * Copyright 1989 - 1994, Julianne Frances Haugh
+ * Copyright (c) 1989 - 1994, Julianne Frances Haugh
+ * Copyright (c) 1996 - 2000, Marek Michałkiewicz
+ * Copyright (c) 2000 - 2006, Tomasz Kłoczko
+ * Copyright (c) 2007 - 2009, Nicolas François
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -10,24 +13,26 @@
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
  *    documentation and/or other materials provided with the distribution.
- * 3. Neither the name of Julianne F. Haugh nor the names of its contributors
- *    may be used to endorse or promote products derived from this software
- *    without specific prior written permission.
+ * 3. The name of the copyright holders or contributors may not be used to
+ *    endorse or promote products derived from this software without
+ *    specific prior written permission.
  *
- * THIS SOFTWARE IS PROVIDED BY JULIE HAUGH AND CONTRIBUTORS ``AS IS'' AND
- * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED. IN NO EVENT SHALL JULIE HAUGH OR CONTRIBUTORS BE LIABLE
- * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
- * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
- * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
- * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
- * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+ * PARTICULAR PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL THE COPYRIGHT
+ * HOLDERS OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
-/* Some parts substantially derived from an ancestor of: */
-/* su for GNU.  Run a shell with substitute user and group IDs.
+
+/* Some parts substantially derived from an ancestor of:
+   su for GNU.  Run a shell with substitute user and group IDs.
+
    Copyright (C) 1992-2003 Free Software Foundation, Inc.
 
    This program is free software; you can redistribute it and/or modify
@@ -41,8 +46,9 @@
    GNU General Public License for more details.
 
    You should have received a copy of the GNU General Public License
-   along with this program; if not, write to the Free Software Foundation,
-   Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  */
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 51 Franklin Street, Fifth Floor,
+   Boston, MA 02110-1301, USA.  */
 
 
 #include <config.h>
@@ -57,31 +63,36 @@
 #include <sys/types.h>
 #include "prototypes.h"
 #include "defines.h"
-#include "exitcodes.h"
 #include "pwauth.h"
 #include "getdef.h"
 #ifdef USE_PAM
 #include "pam_defs.h"
-#endif
+#endif				/* USE_PAM */
+/*@-exitarg@*/
+#include "exitcodes.h"
+
 /*
  * Assorted #defines to control su's behavior
  */
 /*
  * Global variables
  */
+char *Prog;
+/* PID of the child, in case it needs to be killed */
+static pid_t pid_child = 0;
+
 /* not needed by sulog.c anymore */
 static char name[BUFSIZ];
 static char oldname[BUFSIZ];
 
 /* If nonzero, change some environment vars to indicate the user su'd to. */
-static int change_environment;
+static bool change_environment;
 
 #ifdef USE_PAM
 static pam_handle_t *pamh = NULL;
-static int caught = 0;
+static bool caught = false;
 #endif
 
-static char *Prog;
 extern struct passwd pwent;
 
 /*
@@ -94,11 +105,16 @@ extern size_t newenvc;
 
 /* local function prototypes */
 
+static void execve_shell (const char *shellstr,
+                          char *args[],
+                          char *const envp[]);
+static RETSIGTYPE kill_child (int s);
 #ifndef USE_PAM
-
 static RETSIGTYPE die (int);
 static int iswheel (const char *);
+#endif				/* !USE_PAM */
 
+#ifndef USE_PAM
 /*
  * die - set or reset termio modes.
  *
@@ -117,7 +133,7 @@ static RETSIGTYPE die (int killed)
 
 	if (killed) {
 		closelog ();
-		exit (killed);
+		exit (128+killed);
 	}
 }
 
@@ -125,82 +141,147 @@ static int iswheel (const char *username)
 {
 	struct group *grp;
 
-	grp = getgrnam ("wheel");;
-	if (!grp || !grp->gr_mem)
+	grp = getgrnam ("wheel"); /* !USE_PAM, no need for xgetgrnam */
+	if (   (NULL ==grp)
+	    || (NULL == grp->gr_mem)) {
 		return 0;
+	}
 	return is_on_list (grp->gr_mem, username);
 }
 #endif				/* !USE_PAM */
 
+static RETSIGTYPE kill_child (int unused(s))
+{
+	if (0 != pid_child) {
+		(void) kill (pid_child, SIGKILL);
+		(void) fputs (_(" ...killed.\n"), stderr);
+	} else {
+		(void) fputs (_(" ...waiting for child to terminate.\n"),
+		              stderr);
+	}
+	exit (255);
+}
+
 /* borrowed from GNU sh-utils' "su.c" */
-static int restricted_shell (const char *shellstr)
+static bool restricted_shell (const char *shellstr)
 {
 	char *line;
 
 	setusershell ();
 	while ((line = getusershell ()) != NULL) {
-		if (*line != '#' && strcmp (line, shellstr) == 0) {
+		if (('#' != *line) && (strcmp (line, shellstr) == 0)) {
 			endusershell ();
-			return 0;
+			return false;
 		}
 	}
 	endusershell ();
-	return 1;
+	return true;
 }
 
 static void su_failure (const char *tty)
 {
-	sulog (tty, 0, oldname, name);	/* log failed attempt */
+	sulog (tty, false, oldname, name);	/* log failed attempt */
 #ifdef USE_SYSLOG
-	if (getdef_bool ("SYSLOG_SU_ENAB"))
-		SYSLOG ((pwent.pw_uid ? LOG_INFO : LOG_NOTICE,
-			 "- %s %s:%s", tty,
-			 oldname[0] ? oldname : "???", name[0] ? name : "???"));
+	if (getdef_bool ("SYSLOG_SU_ENAB")) {
+		SYSLOG (((0 != pwent.pw_uid) ? LOG_INFO : LOG_NOTICE,
+		         "- %s %s:%s", tty,
+		         ('\0' != oldname[0]) ? oldname : "???",
+		         ('\0' != name[0]) ? name : "???"));
+	}
 	closelog ();
 #endif
 	exit (1);
 }
 
+/*
+ * execve_shell - Execute a shell with execve, or interpret it with
+ * /bin/sh
+ */
+static void execve_shell (const char *shellstr,
+                          char *args[],
+                          char *const envp[])
+{
+	int err;
+	(void) execve (shellstr, (char **) args, envp);
+	err = errno;
+
+	if (access (shellstr, R_OK|X_OK) == 0) {
+		/*
+		 * Assume this is a shell script (with no shebang).
+		 * Interpret it with /bin/sh
+		 */
+		size_t n_args = 0;
+		char **targs;
+		while (NULL != args[n_args]) {
+			n_args++;
+		}
+		targs = (char **) xmalloc ((n_args + 3) * sizeof (args[0]));
+		targs[0] = "sh";
+		targs[1] = "-";
+		targs[2] = xstrdup (shellstr);
+		targs[n_args+2] = NULL;
+		while (1 != n_args) {
+			targs[n_args+1] = args[n_args - 1];
+			n_args--;
+		}
+
+		(void) execve (SHELL, targs, envp);
+	} else {
+		errno = err;
+	}
+}
 
 #ifdef USE_PAM
 /* Signal handler for parent process later */
-static void catch_signals (int sig)
+static void catch_signals (unused int sig)
 {
-	++caught;
+	caught = true;
 }
 
 /* This I ripped out of su.c from sh-utils after the Mandrake pam patch
  * have been applied.  Some work was needed to get it integrated into
  * su.c from shadow.
  */
-static void run_shell (const char *shellstr, char *args[], int doshell,
-		       char *const envp[])
+static void run_shell (const char *shellstr, char *args[], bool doshell,
+                       char *const envp[])
 {
-	int child;
+	pid_t child;
 	sigset_t ourset;
 	int status;
 	int ret;
 
 	child = fork ();
 	if (child == 0) {	/* child shell */
-		pam_end (pamh, PAM_SUCCESS);
+		/*
+		 * PAM_DATA_SILENT is not supported by some modules, and
+		 * there is no strong need to clean up the process space's
+		 * memory since we will either call exec or exit.
+		pam_end (pamh, PAM_SUCCESS | PAM_DATA_SILENT);
+		 */
 
-		if (doshell)
+		if (doshell) {
 			(void) shell (shellstr, (char *) args[0], envp);
-		else
-			(void) execve (shellstr, (char **) args, envp);
+		} else {
+			execve_shell (shellstr, (char **) args, envp);
+		}
+
 		exit (errno == ENOENT ? E_CMD_NOTFOUND : E_CMD_NOEXEC);
-	} else if (child == -1) {
-		(void) fprintf (stderr, "%s: Cannot fork user shell\n", Prog);
+	} else if ((pid_t)-1 == child) {
+		(void) fprintf (stderr,
+		                _("%s: Cannot fork user shell\n"),
+		                Prog);
 		SYSLOG ((LOG_WARN, "Cannot execute %s", shellstr));
 		closelog ();
 		exit (1);
 	}
 	/* parent only */
+	pid_child = child;
 	sigfillset (&ourset);
-	if (sigprocmask (SIG_BLOCK, &ourset, NULL)) {
-		(void) fprintf (stderr, "%s: signal malfunction\n", Prog);
-		caught = 1;
+	if (sigprocmask (SIG_BLOCK, &ourset, NULL) != 0) {
+		(void) fprintf (stderr,
+		                _("%s: signal malfunction\n"),
+		                Prog);
+		caught = true;
 	}
 	if (!caught) {
 		struct sigaction action;
@@ -210,84 +291,89 @@ static void run_shell (const char *shellstr, char *args[], int doshell,
 		action.sa_flags = 0;
 		sigemptyset (&ourset);
 
-		if (sigaddset (&ourset, SIGTERM)
-		    || sigaddset (&ourset, SIGALRM)
-		    || sigaction (SIGTERM, &action, NULL)
-		    || sigprocmask (SIG_UNBLOCK, &ourset, NULL)
+		if (   (sigaddset (&ourset, SIGTERM) != 0)
+		    || (sigaddset (&ourset, SIGALRM) != 0)
+		    || (sigaction (SIGTERM, &action, NULL) != 0)
+		    || (sigprocmask (SIG_UNBLOCK, &ourset, NULL) != 0)
 		    ) {
 			fprintf (stderr,
-				 "%s: signal masking malfunction\n", Prog);
-			caught = 1;
+			         _("%s: signal masking malfunction\n"),
+			         Prog);
+			caught = true;
 		}
 	}
 
 	if (!caught) {
 		do {
-			int pid;
+			pid_t pid;
 
 			pid = waitpid (-1, &status, WUNTRACED);
 
-			if (WIFSTOPPED (status)) {
-				kill (getpid (), SIGSTOP);
-				/* once we get here, we must have resumed */
+			if (((pid_t)-1 != pid) && (0 != WIFSTOPPED (status))) {
+				/* The child (shell) was suspended.
+				 * Suspend su. */
+				kill (getpid (), WSTOPSIG(status));
+				/* wake child when resumed */
 				kill (pid, SIGCONT);
 			}
-		} while (WIFSTOPPED (status));
+		} while (0 != WIFSTOPPED (status));
 	}
 
 	if (caught) {
-		fprintf (stderr, "\nSession terminated, killing shell...");
-		kill (child, SIGTERM);
+		(void) fputs ("\n", stderr);
+		(void) fputs (_("Session terminated, terminating shell..."),
+		              stderr);
+		(void) kill (child, SIGTERM);
 	}
 
 	ret = pam_close_session (pamh, 0);
-	if (ret != PAM_SUCCESS) {
+	if (PAM_SUCCESS != ret) {
 		SYSLOG ((LOG_ERR, "pam_close_session: %s",
 			 pam_strerror (pamh, ret)));
 		fprintf (stderr, _("%s: %s\n"), Prog, pam_strerror (pamh, ret));
-		pam_end (pamh, ret);
+		(void) pam_end (pamh, ret);
 		exit (1);
 	}
 
 	ret = pam_end (pamh, PAM_SUCCESS);
 
 	if (caught) {
-		sleep (2);
-		kill (child, SIGKILL);
-		fprintf (stderr, " ...killed.\n");
-		exit (-1);
+		(void) signal (SIGALRM, kill_child);
+		(void) alarm (2);
+
+		(void) wait (&status);
+		(void) fputs (_(" ...terminated.\n"), stderr);
 	}
 
-	exit (WIFEXITED (status)
-	      ? WEXITSTATUS (status)
-	      : WTERMSIG (status) + 128);
+	exit ((0 != WIFEXITED (status)) ? WEXITSTATUS (status)
+	                                : WTERMSIG (status) + 128);
 }
 #endif
 
 /*
  * usage - print command line syntax and exit
   */
-static void usage (void)
+static void usage (int status)
 {
-	fprintf (stderr, _("Usage: su [options] [LOGIN]\n"
-			   "\n"
-			   "Options:\n"
-			   "  -c, --command COMMAND		pass COMMAND to the invoked shell\n"
-			   "  -h, --help			display this help message and exit\n"
-			   "  -, -l, --login		make the shell a login shell\n"
-			   "  -m, -p,\n"
-			   "  --preserve-environment	do not reset environment variables, and keep\n"
-			   "				the same shell\n"
-			   "  -s, --shell SHELL		use SHELL instead of the default in passwd\n"
-			   "\n"));
-	exit (E_USAGE);
+	fputs (_("Usage: su [options] [LOGIN]\n"
+	         "\n"
+	         "Options:\n"
+	         "  -c, --command COMMAND         pass COMMAND to the invoked shell\n"
+	         "  -h, --help                    display this help message and exit\n"
+	         "  -, -l, --login                make the shell a login shell\n"
+	         "  -m, -p,\n"
+	         "  --preserve-environment        do not reset environment variables, and\n"
+	         "                                keep the same shell\n"
+	         "  -s, --shell SHELL             use SHELL instead of the default in passwd\n"
+	         "\n"), (E_SUCCESS != status) ? stderr : stdout);
+	exit (status);
 }
 
 /*
  * su - switch user id
  *
  *	su changes the user's ids to the values for the specified user.  if
- *	no new user name is specified, "root" is used by default.
+ *	no new user name is specified, "root" or UID 0 is used by default.
  *
  *	Any additional arguments are passed to the user's shell. In
  *	particular, the argument "-c" will cause the next argument to be
@@ -295,15 +381,16 @@ static void usage (void)
  */
 int main (int argc, char **argv)
 {
-	char *cp;
-	const char *tty = 0;	/* Name of tty SU is run from        */
-	int doshell = 0;
-	int fakelogin = 0;
-	int amroot = 0;
+	const char *cp;
+	const char *tty = NULL;	/* Name of tty SU is run from        */
+	bool doshell = false;
+	bool fakelogin = false;
+	bool amroot = false;
 	uid_t my_uid;
-	struct passwd *pw = 0;
+	struct passwd *pw = NULL;
 	char **envp = environ;
-	char *shellstr = 0, *command = 0;
+	char *shellstr = NULL;
+	char *command = NULL;
 
 #ifdef USE_PAM
 	char **envcp;
@@ -311,7 +398,7 @@ int main (int argc, char **argv)
 #else				/* !USE_PAM */
 	int err = 0;
 
-	RETSIGTYPE (*oldsig) ();
+	RETSIGTYPE (*oldsig) (int);
 	int is_console = 0;
 
 	struct spwd *spwd = 0;
@@ -323,11 +410,11 @@ int main (int argc, char **argv)
 
 	sanitize_env ();
 
-	setlocale (LC_ALL, "");
-	bindtextdomain (PACKAGE, LOCALEDIR);
-	textdomain (PACKAGE);
+	(void) setlocale (LC_ALL, "");
+	(void) bindtextdomain (PACKAGE, LOCALEDIR);
+	(void) textdomain (PACKAGE);
 
-	change_environment = 1;
+	change_environment = true;
 
 	/*
 	 * Get the program name. The program name is used as a prefix to
@@ -357,27 +444,17 @@ int main (int argc, char **argv)
 		};
 
 		while ((c =
-			getopt_long (argc, argv, "-c:hlmps:", long_options,
+			getopt_long (argc, argv, "c:hlmps:", long_options,
 				     &option_index)) != -1) {
 			switch (c) {
-			case 1:
-				/* this is not an su option */
-				/* The next arguments are either '-', the
-				 * target name, or arguments to be passed
-				 * to the shell.
-				 */
-				/* rewind the (not yet handled) option */
-				optind--;
-				goto end_su_options;
-				break;	/* NOT REACHED */
 			case 'c':
 				command = optarg;
 				break;
 			case 'h':
-				usage ();
+				usage (E_SUCCESS);
 				break;
 			case 'l':
-				fakelogin = 1;
+				fakelogin = true;
 				break;
 			case 'm':
 			case 'p':
@@ -385,21 +462,23 @@ int main (int argc, char **argv)
 				 * user do not have a restricted shell, or if
 				 * su is called by root.
 				 */
-				change_environment = 0;
+				change_environment = false;
 				break;
 			case 's':
 				shellstr = optarg;
 				break;
 			default:
-				usage ();	/* NOT REACHED */
+				usage (E_USAGE);	/* NOT REACHED */
 			}
 		}
-	      end_su_options:
-		if (optind < argc && !strcmp (argv[optind], "-")) {
-			fakelogin = 1;
+
+		if ((optind < argc) && (strcmp (argv[optind], "-") == 0)) {
+			fakelogin = true;
 			optind++;
-			if (optind < argc && !strcmp (argv[optind], "--"))
+			if (   (optind < argc)
+			    && (strcmp (argv[optind], "--") == 0)) {
 				optind++;
+			}
 		}
 	}
 
@@ -412,11 +491,8 @@ int main (int argc, char **argv)
 	 * Get the tty name. Entries will be logged indicating that the user
 	 * tried to change to the named new user from the current terminal.
 	 */
-	if (isatty (0) && (cp = ttyname (0))) {
-		if (strncmp (cp, "/dev/", 5) == 0)
-			tty = cp + 5;
-		else
-			tty = cp;
+	tty = ttyname (0);
+	if ((isatty (0) != 0) && (NULL != tty)) {
 #ifndef USE_PAM
 		is_console = console (tty);
 #endif
@@ -426,7 +502,8 @@ int main (int argc, char **argv)
 		 */
 		if (!amroot) {
 			fprintf (stderr,
-				 _("%s: must be run from a terminal\n"), Prog);
+			         _("%s: must be run from a terminal\n"),
+			         Prog);
 			exit (1);
 		}
 		tty = "???";
@@ -438,25 +515,42 @@ int main (int argc, char **argv)
 	 * doesn't start with a "-" unless you specify the new user name.
 	 * Any remaining arguments will be passed to the user's login shell.
 	 */
-	if (optind < argc && argv[optind][0] != '-') {
+	if ((optind < argc) && ('-' != argv[optind][0])) {
 		STRFCPY (name, argv[optind++]);	/* use this login id */
-		if (optind < argc && !strcmp (argv[optind], "--"))
+		if ((optind < argc) && (strcmp (argv[optind], "--") == 0)) {
 			optind++;
+		}
+	}
+	if ('\0' == name[0]) {		/* use default user */
+		struct passwd *root_pw = getpwnam ("root");
+		if ((NULL != root_pw) && (0 == root_pw->pw_uid)) {
+			(void) strcpy (name, "root");
+		} else {
+			root_pw = getpwuid (0);
+			if (NULL == root_pw) {
+				SYSLOG ((LOG_CRIT, "There is no UID 0 user."));
+				su_failure (tty);
+			}
+			(void) strcpy (name, root_pw->pw_name);
+		}
 	}
-	if (!name[0])		/* use default user ID */
-		(void) strcpy (name, "root");
 
-	doshell = argc == optind;	/* any arguments remaining? */
-	if (command)
-		doshell = 0;
+	doshell = (argc == optind);	/* any arguments remaining? */
+	if (NULL != command) {
+		doshell = false;
+	}
 
 	/*
 	 * Get the user's real name. The current UID is used to determine
 	 * who has executed su. That user ID must exist.
 	 */
 	pw = get_my_pwent ();
-	if (!pw) {
-		SYSLOG ((LOG_CRIT, "Unknown UID: %u", my_uid));
+	if (NULL == pw) {
+		fprintf (stderr,
+		         _("%s: Cannot determine your user name.\n"),
+		         Prog);
+		SYSLOG ((LOG_WARN, "Cannot determine the user name of the caller (UID %lu)",
+		         (unsigned long) my_uid));
 		su_failure (tty);
 	}
 	STRFCPY (oldname, pw->pw_name);
@@ -467,24 +561,28 @@ int main (int argc, char **argv)
 	 * Sort out the password of user calling su, in case needed later
 	 * -- chris
 	 */
-	if ((spwd = getspnam (oldname)))
+	spwd = getspnam (oldname); /* !USE_PAM, no need for xgetspnam */
+	if (NULL != spwd) {
 		pw->pw_passwd = spwd->sp_pwdp;
+	}
 	oldpass = xstrdup (pw->pw_passwd);
 #endif				/* SU_ACCESS */
 
 #else				/* USE_PAM */
 	ret = pam_start ("su", name, &conv, &pamh);
-	if (ret != PAM_SUCCESS) {
+	if (PAM_SUCCESS != ret) {
 		SYSLOG ((LOG_ERR, "pam_start: error %d", ret);
-			fprintf (stderr, _("%s: pam_start: error %d\n"),
-				 Prog, ret));
+			fprintf (stderr,
+			         _("%s: pam_start: error %d\n"),
+			         Prog, ret));
 		exit (1);
 	}
 
 	ret = pam_set_item (pamh, PAM_TTY, (const void *) tty);
-	if (ret == PAM_SUCCESS)
+	if (PAM_SUCCESS == ret) {
 		ret = pam_set_item (pamh, PAM_RUSER, (const void *) oldname);
-	if (ret != PAM_SUCCESS) {
+	}
+	if (PAM_SUCCESS != ret) {
 		SYSLOG ((LOG_ERR, "pam_set_item: %s",
 			 pam_strerror (pamh, ret)));
 		fprintf (stderr, _("%s: %s\n"), Prog, pam_strerror (pamh, ret));
@@ -502,16 +600,20 @@ int main (int argc, char **argv)
 	 * The password file entries for the user is gotten and the account
 	 * validated.
 	 */
-	if (!(pw = getpwnam (name))) {
+	pw = xgetpwnam (name);
+	if (NULL == pw) {
 		(void) fprintf (stderr, _("Unknown id: %s\n"), name);
 		closelog ();
 		exit (1);
 	}
 #ifndef USE_PAM
 	spwd = NULL;
-	if (strcmp (pw->pw_passwd, SHADOW_PASSWD_STRING) == 0
-	    && (spwd = getspnam (name)))
-		pw->pw_passwd = spwd->sp_pwdp;
+	if (strcmp (pw->pw_passwd, SHADOW_PASSWD_STRING) == 0) {
+		spwd = getspnam (name); /* !USE_PAM, no need for xgetspnam */
+		if (NULL != spwd) {
+			pw->pw_passwd = spwd->sp_pwdp;
+		}
+	}
 #endif				/* !USE_PAM */
 	pwent = *pw;
 
@@ -526,35 +628,61 @@ int main (int argc, char **argv)
 	 * (note: in the case of a subsystem, the shell will be restricted,
 	 *        and this won't be executed on the first pass)
 	 */
-	if (fakelogin && change_environment) {
+	if (change_environment && fakelogin) {
 		/*
 		 * The terminal type will be left alone if it is present in
 		 * the environment already.
 		 */
-		if ((cp = getenv ("TERM")))
+		cp = getenv ("TERM");
+		if (NULL != cp) {
 			addenv ("TERM", cp);
+		}
+
+		/*
+		 * For some terminals COLORTERM seems to be the only way
+		 * for checking for that specific terminal. For instance,
+		 * gnome-terminal sets its TERM as "xterm" but its
+		 * COLORTERM as "gnome-terminal". The COLORTERM variable
+		 * is also of use when running GNU screen since it sets
+		 * TERM to "screen" but doesn't touch COLORTERM.
+		 */
+		cp = getenv ("COLORTERM");
+		if (NULL != cp) {
+			addenv ("COLORTERM", cp);
+		}
+
 #ifndef USE_PAM
-		if ((cp = getdef_str ("ENV_TZ")))
-			addenv (*cp == '/' ? tz (cp) : cp, NULL);
+		cp = getdef_str ("ENV_TZ");
+		if (NULL != cp) {
+			addenv (('/' == *cp) ? tz (cp) : cp, NULL);
+		}
 
 		/*
 		 * The clock frequency will be reset to the login value if required
 		 */
-		if ((cp = getdef_str ("ENV_HZ")))
+		cp = getdef_str ("ENV_HZ");
+		if (NULL != cp) {
 			addenv (cp, NULL);	/* set the default $HZ, if one */
+		}
+#endif				/* !USE_PAM */
 
 		/*
 		 * Also leave DISPLAY and XAUTHORITY if present, else
 		 * pam_xauth will not work.
 		 */
-		if ((cp = getenv ("DISPLAY")))
+		cp = getenv ("DISPLAY");
+		if (NULL != cp) {
 			addenv ("DISPLAY", cp);
-		if ((cp = getenv ("XAUTHORITY")))
+		}
+		cp = getenv ("XAUTHORITY");
+		if (NULL != cp) {
 			addenv ("XAUTHORITY", cp);
-#endif				/* !USE_PAM */
+		}
 	} else {
-		while (*envp)
-			addenv (*envp++, NULL);
+		while (NULL != *envp) {
+			addenv (*envp, NULL);
+			envp++;
+		}
 	}
 
 #ifndef USE_PAM
@@ -577,10 +705,12 @@ int main (int argc, char **argv)
 	 */
 
 	if (!amroot) {
-		if (pwent.pw_uid == 0 && getdef_bool ("SU_WHEEL_ONLY")
+		if (   (0 == pwent.pw_uid)
+		    && getdef_bool ("SU_WHEEL_ONLY")
 		    && !iswheel (oldname)) {
 			fprintf (stderr,
-				 _("You are not authorized to su %s\n"), name);
+			         _("You are not authorized to su %s\n"),
+			         name);
 			exit (1);
 		}
 #ifdef SU_ACCESS
@@ -596,7 +726,8 @@ int main (int argc, char **argv)
 			break;
 		default:	/* access denied (-1) or unexpected value */
 			fprintf (stderr,
-				 _("You are not authorized to su %s\n"), name);
+			         _("You are not authorized to su %s\n"),
+			         name);
 			exit (1);
 		}
 #endif				/* SU_ACCESS */
@@ -607,58 +738,67 @@ int main (int argc, char **argv)
 	 * use the current SHELL.
 	 * (unless another shell is required by the command line)
 	 */
-	if (shellstr == NULL && change_environment == 0)
+	if ((NULL == shellstr) && !change_environment) {
 		shellstr = getenv ("SHELL");
+	}
 	/* For users with non null UID, if this user has a restricted
 	 * shell, the shell must be the one specified in /etc/passwd
 	 */
-	if (shellstr != NULL && !amroot && restricted_shell (pwent.pw_shell))
+	if (   (NULL != shellstr)
+	    && !amroot
+	    && restricted_shell (pwent.pw_shell)) {
 		shellstr = NULL;
+	}
 	/* If the shell is not set at this time, use the shell specified
 	 * in /etc/passwd.
 	 */
-	if (shellstr == NULL)
+	if (NULL == shellstr) {
 		shellstr = (char *) strdup (pwent.pw_shell);
+	}
 
 	/*
 	 * Set the default shell.
 	 */
-	if (shellstr == NULL || shellstr[0] == '\0')
-		shellstr = "/bin/sh";
+	if ((NULL == shellstr) || ('\0' == shellstr[0])) {
+		shellstr = SHELL;
+	}
 
-	signal (SIGINT, SIG_IGN);
-	signal (SIGQUIT, SIG_IGN);
+	(void) signal (SIGINT, SIG_IGN);
+	(void) signal (SIGQUIT, SIG_IGN);
 #ifdef USE_PAM
 	ret = pam_authenticate (pamh, 0);
-	if (ret != PAM_SUCCESS) {
+	if (PAM_SUCCESS != ret) {
 		SYSLOG ((LOG_ERR, "pam_authenticate: %s",
 			 pam_strerror (pamh, ret)));
 		fprintf (stderr, _("%s: %s\n"), Prog, pam_strerror (pamh, ret));
-		pam_end (pamh, ret);
+		(void) pam_end (pamh, ret);
 		su_failure (tty);
 	}
 
 	ret = pam_acct_mgmt (pamh, 0);
-	if (ret != PAM_SUCCESS) {
+	if (PAM_SUCCESS != ret) {
 		if (amroot) {
-			fprintf (stderr, _("%s: %s\n(Ignored)\n"), Prog,
-				 pam_strerror (pamh, ret));
-		} else if (ret == PAM_NEW_AUTHTOK_REQD) {
+			fprintf (stderr,
+			         _("%s: %s\n(Ignored)\n"),
+			         Prog, pam_strerror (pamh, ret));
+		} else if (PAM_NEW_AUTHTOK_REQD == ret) {
 			ret = pam_chauthtok (pamh, PAM_CHANGE_EXPIRED_AUTHTOK);
-			if (ret != PAM_SUCCESS) {
+			if (PAM_SUCCESS != ret) {
 				SYSLOG ((LOG_ERR, "pam_chauthtok: %s",
 					 pam_strerror (pamh, ret)));
-				fprintf (stderr, _("%s: %s\n"), Prog,
-					 pam_strerror (pamh, ret));
-				pam_end (pamh, ret);
+				fprintf (stderr,
+				         _("%s: %s\n"),
+				         Prog, pam_strerror (pamh, ret));
+				(void) pam_end (pamh, ret);
 				su_failure (tty);
 			}
 		} else {
 			SYSLOG ((LOG_ERR, "pam_acct_mgmt: %s",
 				 pam_strerror (pamh, ret)));
-			fprintf (stderr, _("%s: %s\n"), Prog,
-				 pam_strerror (pamh, ret));
-			pam_end (pamh, ret);
+			fprintf (stderr,
+			         _("%s: %s\n"),
+			         Prog, pam_strerror (pamh, ret));
+			(void) pam_end (pamh, ret);
 			su_failure (tty);
 		}
 	}
@@ -676,10 +816,11 @@ int main (int argc, char **argv)
 	 */
 	if (!amroot && pw_auth (pwent.pw_passwd, name, PW_SU, (char *) 0)) {
 		SYSLOG ((pwent.pw_uid ? LOG_NOTICE : LOG_WARN,
-			 "Authentication failed for %s", name));
+		         "Authentication failed for %s", name));
+		fprintf(stderr, _("%s: Authentication failure\n"), Prog);
 		su_failure (tty);
 	}
-	signal (SIGQUIT, oldsig);
+	(void) signal (SIGQUIT, oldsig);
 
 	/*
 	 * Check to see if the account is expired. root gets to ignore any
@@ -687,15 +828,19 @@ int main (int argc, char **argv)
 	 * expired password.
 	 */
 	if (!amroot) {
-		if (!spwd)
+		if (NULL == spwd) {
 			spwd = pwd_to_spwd (&pwent);
+		}
 
 		if (expire (&pwent, spwd)) {
+			/* !USE_PAM, no need for xgetpwnam */
 			struct passwd *pwd = getpwnam (name);
 
+			/* !USE_PAM, no need for xgetspnam */
 			spwd = getspnam (name);
-			if (pwd)
+			if (NULL != pwd) {
 				pwent = *pwd;
+			}
 		}
 	}
 
@@ -707,35 +852,39 @@ int main (int argc, char **argv)
 	 */
 	if (!amroot) {
 		if (!isttytime (pwent.pw_name, "SU", time ((time_t *) 0))) {
-			SYSLOG ((pwent.pw_uid ? LOG_WARN : LOG_CRIT,
-				 "SU by %s to restricted account %s",
-				 oldname, name));
+			SYSLOG (((0 != pwent.pw_uid) ? LOG_WARN : LOG_CRIT,
+			         "SU by %s to restricted account %s",
+			         oldname, name));
+			fprintf (stderr,
+			         _("%s: You are not authorized to su at that time\n"),
+			         Prog);
 			su_failure (tty);
 		}
 	}
 #endif				/* !USE_PAM */
 
-	signal (SIGINT, SIG_DFL);
-	signal (SIGQUIT, SIG_DFL);
+	(void) signal (SIGINT, SIG_DFL);
+	(void) signal (SIGQUIT, SIG_DFL);
 
 	cp = getdef_str ((pwent.pw_uid == 0) ? "ENV_SUPATH" : "ENV_PATH");
-	if (!cp) {
+	if (NULL == cp) {
 		addenv ("PATH=/bin:/usr/bin", NULL);
-	} else if (strchr (cp, '=')) {
+	} else if (strchr (cp, '=') != NULL) {
 		addenv (cp, NULL);
 	} else {
 		addenv ("PATH", cp);
 	}
 
-	if (getenv ("IFS"))	/* don't export user IFS ... */
+	if (getenv ("IFS") != NULL) {	/* don't export user IFS ... */
 		addenv ("IFS= \t\n", NULL);	/* ... instead, set a safe IFS */
+	}
 
 	/*
 	 * Even if --shell is specified, the subsystem login test is based on
 	 * the shell specified in /etc/passwd (not the one specified with
 	 * --shell, which will be the one executed in the chroot later).
 	 */
-	if (pwent.pw_shell[0] == '*') {	/* subsystem root required */
+	if ('*' == pwent.pw_shell[0]) {	/* subsystem root required */
 		pwent.pw_shell++;	/* skip the '*' */
 		subsystem (&pwent);	/* figure out what to execute */
 		endpwent ();
@@ -743,18 +892,20 @@ int main (int argc, char **argv)
 		goto top;
 	}
 
-	sulog (tty, 1, oldname, name);	/* save SU information */
+	sulog (tty, true, oldname, name);	/* save SU information */
 	endpwent ();
 	endspent ();
 #ifdef USE_SYSLOG
-	if (getdef_bool ("SYSLOG_SU_ENAB"))
+	if (getdef_bool ("SYSLOG_SU_ENAB")) {
 		SYSLOG ((LOG_INFO, "+ %s %s:%s", tty,
-			 oldname[0] ? oldname : "???", name[0] ? name : "???"));
+		         ('\0' != oldname[0]) ? oldname : "???",
+		         ('\0' != name[0]) ? name : "???"));
+	}
 #endif
 
 #ifdef USE_PAM
 	/* set primary group id and supplementary groups */
-	if (setup_groups (&pwent)) {
+	if (setup_groups (&pwent) != 0) {
 		pam_end (pamh, PAM_ABORT);
 		exit (1);
 	}
@@ -764,20 +915,20 @@ int main (int argc, char **argv)
 	 * and much more, depending on the configured modules
 	 */
 	ret = pam_setcred (pamh, PAM_ESTABLISH_CRED);
-	if (ret != PAM_SUCCESS) {
+	if (PAM_SUCCESS != ret) {
 		SYSLOG ((LOG_ERR, "pam_setcred: %s", pam_strerror (pamh, ret)));
 		fprintf (stderr, _("%s: %s\n"), Prog, pam_strerror (pamh, ret));
-		pam_end (pamh, ret);
+		(void) pam_end (pamh, ret);
 		exit (1);
 	}
 
 	ret = pam_open_session (pamh, 0);
-	if (ret != PAM_SUCCESS) {
+	if (PAM_SUCCESS != ret) {
 		SYSLOG ((LOG_ERR, "pam_open_session: %s",
 			 pam_strerror (pamh, ret)));
 		fprintf (stderr, _("%s: %s\n"), Prog, pam_strerror (pamh, ret));
 		pam_setcred (pamh, PAM_DELETE_CRED);
-		pam_end (pamh, ret);
+		(void) pam_end (pamh, ret);
 		exit (1);
 	}
 
@@ -790,8 +941,8 @@ int main (int argc, char **argv)
 
 		/* update environment with all pam set variables */
 		envcp = pam_getenvlist (pamh);
-		if (envcp) {
-			while (*envcp) {
+		if (NULL != envcp) {
+			while (NULL != *envcp) {
 				addenv (*envcp, NULL);
 				envcp++;
 			}
@@ -799,21 +950,23 @@ int main (int argc, char **argv)
 	}
 
 	/* become the new user */
-	if (change_uid (&pwent)) {
+	if (change_uid (&pwent) != 0) {
 		pam_close_session (pamh, 0);
 		pam_setcred (pamh, PAM_DELETE_CRED);
-		pam_end (pamh, PAM_ABORT);
+		(void) pam_end (pamh, PAM_ABORT);
 		exit (1);
 	}
 #else				/* !USE_PAM */
 	environ = newenvp;	/* make new environment active */
 
 	/* no limits if su from root (unless su must fake login's behavior) */
-	if (!amroot || fakelogin)
+	if (!amroot || fakelogin) {
 		setup_limits (&pwent);
+	}
 
-	if (setup_uid_gid (&pwent, is_console))
+	if (setup_uid_gid (&pwent, is_console) != 0) {
 		exit (1);
+	}
 #endif				/* !USE_PAM */
 
 	if (change_environment) {
@@ -845,20 +998,22 @@ int main (int argc, char **argv)
 		char *arg0;
 
 		cp = getdef_str ("SU_NAME");
-		if (!cp)
+		if (NULL == cp) {
 			cp = Basename (shellstr);
+		}
 
 		arg0 = xmalloc (strlen (cp) + 2);
 		arg0[0] = '-';
 		strcpy (arg0 + 1, cp);
 		cp = arg0;
-	} else
+	} else {
 		cp = Basename (shellstr);
+	}
 
 	if (!doshell) {
 		/* Position argv to the remaining arguments */
 		argv += optind;
-		if (command) {
+		if (NULL != command) {
 			argv -= 2;
 			argv[0] = "-c";
 			argv[1] = command;
@@ -869,22 +1024,23 @@ int main (int argc, char **argv)
 		 */
 		argv[-1] = shellstr;
 #ifndef USE_PAM
-		(void) execve (shellstr, &argv[-1], environ);
+		execve_shell (shellstr, &argv[-1], environ);
 		err = errno;
-		(void) fprintf (stderr, _("No shell\n"));
+		(void) fputs (_("No shell\n"), stderr);
 		SYSLOG ((LOG_WARN, "Cannot execute %s", shellstr));
 		closelog ();
-		exit (err == ENOENT ? E_CMD_NOTFOUND : E_CMD_NOEXEC);
+		exit ((ENOENT == err) ? E_CMD_NOTFOUND : E_CMD_NOEXEC);
 #else
-		run_shell (shellstr, &argv[-1], 0, environ);	/* no return */
+		run_shell (shellstr, &argv[-1], false, environ); /* no return */
 #endif
 	}
 #ifndef USE_PAM
 	err = shell (shellstr, cp, environ);
-	exit (err == ENOENT ? E_CMD_NOTFOUND : E_CMD_NOEXEC);
+	exit ((ENOENT == err) ? E_CMD_NOTFOUND : E_CMD_NOEXEC);
 #else
-	run_shell (shellstr, &cp, 1, environ);
+	run_shell (shellstr, &cp, true, environ);
 #endif
 	/* NOT REACHED */
 	exit (1);
 }
+