From 10b8aaa1564b723a005b53acc069df71313f4cac Mon Sep 17 00:00:00 2001
From: Nalin Dahyabhai <nalin@redhat.com>
Date: May 07 2020 18:43:54 +0000
Subject: CVE-2020-10737: defer setting permissions on newly-created home directories


mkhomedir: add a patch from Matthias Gerstner of the SUSE security team
to defer setting permissions on newly-created home directories until
after we've finished populating them, to prevent possible interference
and attacks from the user who will eventually be given access to the new
home directory, while it's still being populated (CVE-2020-10737).

Author: Matthias Gerstner
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>

---

diff --git a/src/mkhomedir.c b/src/mkhomedir.c
index 98ce45b..1c0d8e4 100644
--- a/src/mkhomedir.c
+++ b/src/mkhomedir.c
@@ -61,9 +61,11 @@ static mode_t override_umask;
  * identical as possible a copy in the destination tree. */
 static int
 copy_single_item(const char *source, const struct stat *sb,
-		 int flag, struct FTW *unused_s)
+		 int flag, struct FTW *status)
 {
-	int sfd, dfd, i;
+	uid_t uid = pwd->pw_uid;
+	gid_t gid = pwd->pw_gid;
+	int sfd, dfd, i, res;
 	char target[PATH_MAX + 1], newpath[PATH_MAX + 1];
 	unsigned char buf[BUFSIZ];
 	/* Generate the name of the new item. */
@@ -152,12 +154,24 @@ copy_single_item(const char *source, const struct stat *sb,
 		}
 		return 0;
 	case FTW_D:
+		/* It's the home directory itself. Don't give it to the
+		 * target user just yet to avoid potential race conditions
+		 * involving symlink attacks when we copy over the skeleton
+		 * tree. */
+		if (status->level == 0) {
+			uid = 0;
+			gid = 0;
+		}
+
 		/* It's a directory.  Make one with the same name and
 		 * permissions, but owned by the target user. */
-		if ((oddjob_selinux_mkdir(newpath,
-					  sb->st_mode & ~override_umask,
-					  pwd->pw_uid, pwd->pw_gid) != 0) &&
-		    (errno != EEXIST)) {
+		res = oddjob_selinux_mkdir(newpath,
+					   sb->st_mode & ~override_umask,
+					   uid, gid);
+
+		/* on unexpected errors, or if the home directory itself
+		 * suddenly already exists, abort the copy operation. */
+		if (res != 0 && (errno != EEXIST || status->level == 0)) {
 			return HANDLER_FAILURE;
 		}
 		return 0;
@@ -220,8 +234,14 @@ mkhomedir(const char *user, int flags)
 			}
 			/* Walk the template tree and make a copy. */
 			if (flags & FLAG_POPULATE) {
-				return nftw(get_skel_dir(), copy_single_item, 5,
-					    FTW_PHYS);
+				int res = nftw(get_skel_dir(), copy_single_item, 5,
+					       FTW_PHYS);
+				/* only now give ownership to the target user */
+				if (res == 0) {
+					res = chown(pwd->pw_dir, pwd->pw_uid, pwd->pw_gid);
+				}
+
+				return res;
 			} else {
 				if (stat(skel, &st) != 0) {
 					st.st_mode = S_IRWXU;