]> git.ipfire.org Git - thirdparty/util-linux.git/commitdiff
chsh, chfn, vipw: fix filenames collision
authorKarel Zak <kzak@redhat.com>
Mon, 24 Aug 2015 08:05:55 +0000 (10:05 +0200)
committerKarel Zak <kzak@redhat.com>
Mon, 24 Aug 2015 08:05:55 +0000 (10:05 +0200)
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
lib/fileutils.c
login-utils/chfn.c
login-utils/chsh.c
login-utils/setpwnam.c
login-utils/setpwnam.h
login-utils/vipw.c

index e0e2ecee129e0e66bb835383f3e08627974e3152..be6d1f7f9b094761dc151c73ad6f3ca1a6ac3840 100644 (file)
@@ -7,14 +7,14 @@
 
 #include "c.h"
 
-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;
 
index 81b38ad3a7013275f21958b3ab1833674ba338a8..bf8e60a4b3e6682a64407541fbcaf386bfeaf318 100644 (file)
 
 /* 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);
index ac0a3cbde29585049b6d48fab1fcf923c53bcb5f..b1c7ea25a22876188b61eadbbd4bdffbde9f4458 100644 (file)
@@ -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. */
        ctl->pw->pw_gecos = gecos;
-       if (setpwnam(ctl->pw) < 0) {
+       if (setpwnam(ctl->pw, ".chfn") < 0) {
                warn("setpwnam failed");
 #endif
                printf(_
index f4455d7e2cc31a5dd6521f724eca150e9fe20afc..d74a1f0f10a76a588218a2b02bcd6fd61daa9fb6 100644 (file)
@@ -323,7 +323,7 @@ int main(int argc, char **argv)
                errx(EXIT_FAILURE, _("Shell *NOT* changed.  Try again later."));
 #else
        pw->pw_shell = info.shell;
-       if (setpwnam(pw) < 0)
+       if (setpwnam(pw, ".chsh") < 0)
                err(EXIT_FAILURE, _("setpwnam failed\n"
                        "Shell *NOT* changed.  Try again later."));
 #endif
index 79f3299d45192ba98ee5024469c2d26be9313078..9f39d018129a34d8ca0fa71be6ce72b420e2905a 100644 (file)
@@ -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 */
index 7d69445e8b4e0603f18f57adc2a1a32e9688afd8..95785923f6351778004f3c213457c7abab03d928 100644 (file)
@@ -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 */
index 9fb25502c9a8392d13998238b45d00e980e32d49..a3c932f1651f98d92b04d58c2bc9f19ece151193 100644 (file)
@@ -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"));
        }