From bde91c85bdc77975155058276f99d2e0f5eab5a9 Mon Sep 17 00:00:00 2001
From: Karel Zak <kzak@redhat.com>
Date: Mon, 24 Aug 2015 10:05:55 +0200
Subject: [PATCH] chsh, chfn, vipw: fix filenames collision

The utils when compiled WITHOUT libuser then mkostemp()ing
"/etc/%s.XXXXXX" where the filename prefix is argv[0] basename.

An attacker could repeatedly execute the util with modified argv[0]
and after many many attempts mkostemp() may generate suffix which
makes sense. The result maybe temporary file with name like rc.status
ld.so.preload or krb5.keytab, etc.

Note that distros usually use libuser based ch{sh,fn} or stuff from
shadow-utils.

It's probably very minor security bug.

Addresses: CVE-2015-5224
Signed-off-by: Karel Zak <kzak@redhat.com>
---
 include/fileutils.h    |  6 +++---
 lib/fileutils.c        | 21 +++++++--------------
 login-utils/chfn.c     |  2 +-
 login-utils/chsh.c     |  2 +-
 login-utils/setpwnam.c |  5 ++---
 login-utils/setpwnam.h |  6 +++++-
 login-utils/vipw.c     |  3 +--
 7 files changed, 20 insertions(+), 25 deletions(-)

diff --git a/include/fileutils.h b/include/fileutils.h
index e0e2ece..be6d1f7 100644
--- a/include/fileutils.h
+++ b/include/fileutils.h
@@ -7,14 +7,14 @@
 #ifndef UTIL_LINUX_FILEUTILS
 #define UTIL_LINUX_FILEUTILS
 
-extern int xmkstemp(char **tmpname, char *dir);
+extern int xmkstemp(char **tmpname, const char *dir, const char *prefix);
 
-static inline FILE *xfmkstemp(char **tmpname, char *dir)
+static inline FILE *xfmkstemp(char **tmpname, const char *dir, const char *prefix)
 {
 	int fd;
 	FILE *ret;
 
-	fd = xmkstemp(tmpname, dir);
+	fd = xmkstemp(tmpname, dir, prefix);
 	if (fd == -1)
 		return NULL;
 
diff --git a/lib/fileutils.c b/lib/fileutils.c
index 81b38ad..bf8e60a 100644
--- a/lib/fileutils.c
+++ b/lib/fileutils.c
@@ -15,27 +15,20 @@
 
 /* Create open temporary file in safe way.  Please notice that the
  * file permissions are -rw------- by default. */
-int xmkstemp(char **tmpname, char *dir)
+int xmkstemp(char **tmpname, const char *dir, const char *prefix)
 {
 	char *localtmp;
-	char *tmpenv;
+	const char *tmpenv;
 	mode_t old_mode;
 	int fd, rc;
 
 	/* Some use cases must be capable of being moved atomically
 	 * with rename(2), which is the reason why dir is here.  */
-	if (dir != NULL)
-		tmpenv = dir;
-	else
-		tmpenv = getenv("TMPDIR");
-
-	if (tmpenv)
-		rc = asprintf(&localtmp, "%s/%s.XXXXXX", tmpenv,
-			  program_invocation_short_name);
-	else
-		rc = asprintf(&localtmp, "%s/%s.XXXXXX", _PATH_TMP,
-			  program_invocation_short_name);
+	tmpenv = dir ? dir : getenv("TMPDIR");
+	if (!tmpenv)
+		tmpenv = _PATH_TMP;
 
+	rc = asprintf(&localtmp, "%s/%s.XXXXXX", tmpenv, prefix);
 	if (rc < 0)
 		return -1;
 
@@ -107,7 +100,7 @@ int main(void)
 {
 	FILE *f;
 	char *tmpname;
-	f = xfmkstemp(&tmpname, NULL);
+	f = xfmkstemp(&tmpname, NULL, "test");
 	unlink(tmpname);
 	free(tmpname);
 	fclose(f);
diff --git a/login-utils/chfn.c b/login-utils/chfn.c
index ac0a3cb..b1c7ea2 100644
--- a/login-utils/chfn.c
+++ b/login-utils/chfn.c
@@ -373,7 +373,7 @@ static int save_new_data(struct chfn_control *ctl)
 #else /* HAVE_LIBUSER */
 	/* write the new struct passwd to the passwd file. */
 	pinfo->pw->pw_gecos = gecos;
-	if (setpwnam(pinfo->pw) < 0) {
+	if (setpwnam(pinfo->pw, ".chfn") < 0) {
 		warn("setpwnam failed");
 #endif
 		printf(_
diff --git a/login-utils/chsh.c b/login-utils/chsh.c
index f4455d7..d74a1f0 100644
--- a/login-utils/chsh.c
+++ b/login-utils/chsh.c
@@ -323,7 +323,7 @@ int main(int argc, char **argv)
 		errx(EXIT_FAILURE, _("Shell *NOT* changed.  Try again later."));
 #else
 	pw->pw_shell = shell;
-	if (setpwnam(pw) < 0)
+	if (setpwnam(pw, ".chsh") < 0)
 		err(EXIT_FAILURE, _("setpwnam failed\n"
 			"Shell *NOT* changed.  Try again later."));
 #endif
diff --git a/login-utils/setpwnam.c b/login-utils/setpwnam.c
index 79f3299..9f39d01 100644
--- a/login-utils/setpwnam.c
+++ b/login-utils/setpwnam.c
@@ -71,7 +71,7 @@ static void pw_init(void);
  *	If the given username exists in the passwd file, the entry is
  *	replaced with the given entry.
  */
-int setpwnam(struct passwd *pwd)
+int setpwnam(struct passwd *pwd, const char *prefix)
 {
 	FILE *fp = NULL, *pwf = NULL;
 	int save_errno;
@@ -81,11 +81,10 @@ int setpwnam(struct passwd *pwd)
 	int contlen, rc;
 	char *linebuf = NULL;
 	char *tmpname = NULL;
-	char *atomic_dir = "/etc";
 
 	pw_init();
 
-	if ((fp = xfmkstemp(&tmpname, atomic_dir)) == NULL)
+	if ((fp = xfmkstemp(&tmpname, "/etc", prefix)) == NULL)
 		return -1;
 
 	/* ptmp should be owned by root.root or root.wheel */
diff --git a/login-utils/setpwnam.h b/login-utils/setpwnam.h
index 7d69445..9578592 100644
--- a/login-utils/setpwnam.h
+++ b/login-utils/setpwnam.h
@@ -11,6 +11,8 @@
  *  published by the Free Software Foundation; either version 2 of the
  *  License, or (at your option) any later version.
  */
+#ifndef UTIL_LINUX_SETPWNAM_H
+#define UTIL_LINUX_SETPWNAM_H
 
 #include "pathnames.h"
 
@@ -26,4 +28,6 @@
 # define SGROUP_FILE	"/tmp/gshadow"
 #endif
 
-extern int setpwnam (struct passwd *pwd);
+extern int setpwnam (struct passwd *pwd, const char *prefix);
+
+#endif /* UTIL_LINUX_SETPWNAM_H */
diff --git a/login-utils/vipw.c b/login-utils/vipw.c
index 9fb2550..a3c932f 100644
--- a/login-utils/vipw.c
+++ b/login-utils/vipw.c
@@ -135,9 +135,8 @@ static FILE * pw_tmpfile(int lockfd)
 {
 	FILE *fd;
 	char *tmpname = NULL;
-	char *dir = "/etc";
 
-	if ((fd = xfmkstemp(&tmpname, dir)) == NULL) {
+	if ((fd = xfmkstemp(&tmpname, "/etc", ".vipw")) == NULL) {
 		ulckpwdf();
 		err(EXIT_FAILURE, _("can't open temporary file"));
 	}