Avoid races in chown_tree()

Use *at() functions to pin the directory operating in to avoid being
redirected by unprivileged users replacing parts of paths by symlinks to
privileged files.

libmisc/chowndir.c

@@ -17,45 +17,28 @@
 #include "defines.h"
 #include <fcntl.h>
 #include <stdio.h>
-/*
- * chown_tree - change ownership of files in a directory tree
- *
- *	chown_dir() walks a directory tree and changes the ownership
- *	of all files owned by the provided user ID.
- *
- *	Only files owned (resp. group-owned) by old_uid (resp. by old_gid)
- *	will have their ownership (resp. group-ownership) modified, unless
- *	old_uid (resp. old_gid) is set to -1.
- *
- *	new_uid and new_gid can be set to -1 to indicate that no owner or
- *	group-owner shall be changed.
- */
-int chown_tree (const char *root,
+#include <unistd.h>
+
+static int chown_tree_at (int at_fd,
+                const char *path,
                 uid_t old_uid,
                 uid_t new_uid,
                 gid_t old_gid,
                 gid_t new_gid)
 {
-	char *new_name;
-	size_t new_name_len;
-	int rc = 0;
-	struct dirent *ent;
-	struct stat sb;
 	DIR *dir;
+	const struct dirent *ent;
+	struct stat dir_sb;
+	int dir_fd, rc = 0;
 
-	new_name = malloc (1024);
-	if (NULL == new_name) {
+	dir_fd = openat (at_fd, path, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC);
+	if (dir_fd < 0) {
 		return -1;
 	}
-	new_name_len = 1024;
 
-	/*
-	 * Make certain the directory exists.  This routine is called
-	 * directly by the invoker, or recursively.
-	 */
-
-	if (access (root, F_OK) != 0) {
-		free (new_name);
+	dir = fdopendir (dir_fd);
+	if (!dir) {
+		(void) close (dir_fd);
 		return -1;
 	}
 
@@ -65,68 +48,34 @@ int chown_tree (const char *root,
 	 * recursively.  If not, it is checked to see if an ownership
 	 * shall be changed.
 	 */
-
-	dir = opendir (root);
-	if (NULL == dir) {
-		free (new_name);
-		return -1;
-	}
-
 	while ((ent = readdir (dir))) {
-		size_t ent_name_len;
 		uid_t tmpuid = (uid_t) -1;
 		gid_t tmpgid = (gid_t) -1;
+		struct stat ent_sb;
 
 		/*
 		 * Skip the "." and ".." entries
 		 */
-
 		if (   (strcmp (ent->d_name, ".") == 0)
 		    || (strcmp (ent->d_name, "..") == 0)) {
 			continue;
 		}
 
-		/*
-		 * Make the filename for both the source and the
-		 * destination files.
-		 */
-
-		ent_name_len = strlen (root) + strlen (ent->d_name) + 2;
-		if (ent_name_len > new_name_len) {
-			/*@only@*/char *tmp = realloc (new_name, ent_name_len);
-			if (NULL == tmp) {
-				rc = -1;
-				break;
-			}
-			new_name = tmp;
-			new_name_len = ent_name_len;
-		}
-
-		(void) snprintf (new_name, new_name_len, "%s/%s", root, ent->d_name);
-
-		/* Don't follow symbolic links! */
-		if (LSTAT (new_name, &sb) == -1) {
-			continue;
+		rc = fstatat (dirfd(dir), ent->d_name, &ent_sb, AT_SYMLINK_NOFOLLOW);
+		if (rc < 0) {
+			break;
 		}
 
-		if (S_ISDIR (sb.st_mode) && !S_ISLNK (sb.st_mode)) {
-
+		if (S_ISDIR (ent_sb.st_mode)) {
 			/*
 			 * Do the entire subdirectory.
 			 */
-
-			rc = chown_tree (new_name, old_uid, new_uid,
-			                 old_gid, new_gid);
+			rc = chown_tree_at (dirfd(dir), ent->d_name, old_uid, new_uid, old_gid, new_gid);
 			if (0 != rc) {
 				break;
 			}
 		}
-#ifndef HAVE_LCHOWN
-		/* don't use chown (follows symbolic links!) */
-		if (S_ISLNK (sb.st_mode)) {
-			continue;
-		}
-#endif
+
 		/*
 		 * By default, the IDs are not changed (-1).
 		 *
@@ -136,43 +85,62 @@ int chown_tree (const char *root,
 		 * If the file is not group-owned by the group, the
 		 * group-owner is not changed.
 		 */
-		if (((uid_t) -1 == old_uid) || (sb.st_uid == old_uid)) {
+		if (((uid_t) -1 == old_uid) || (ent_sb.st_uid == old_uid)) {
 			tmpuid = new_uid;
 		}
-		if (((gid_t) -1 == old_gid) || (sb.st_gid == old_gid)) {
+		if (((gid_t) -1 == old_gid) || (ent_sb.st_gid == old_gid)) {
 			tmpgid = new_gid;
 		}
 		if (((uid_t) -1 != tmpuid) || ((gid_t) -1 != tmpgid)) {
-			rc = LCHOWN (new_name, tmpuid, tmpgid);
+			rc = fchownat (dirfd(dir), ent->d_name, tmpuid, tmpgid, AT_SYMLINK_NOFOLLOW);
 			if (0 != rc) {
 				break;
 			}
 		}
 	}
 
-	free (new_name);
-	(void) closedir (dir);
-
 	/*
 	 * Now do the root of the tree
 	 */
-
-	if ((0 == rc) && (stat (root, &sb) == 0)) {
+	if ((0 == rc) && (fstat (dirfd(dir), &dir_sb) == 0)) {
 		uid_t tmpuid = (uid_t) -1;
 		gid_t tmpgid = (gid_t) -1;
-		if (((uid_t) -1 == old_uid) || (sb.st_uid == old_uid)) {
+		if (((uid_t) -1 == old_uid) || (dir_sb.st_uid == old_uid)) {
 			tmpuid = new_uid;
 		}
-		if (((gid_t) -1 == old_gid) || (sb.st_gid == old_gid)) {
+		if (((gid_t) -1 == old_gid) || (dir_sb.st_gid == old_gid)) {
 			tmpgid = new_gid;
 		}
 		if (((uid_t) -1 != tmpuid) || ((gid_t) -1 != tmpgid)) {
-			rc = LCHOWN (root, tmpuid, tmpgid);
+			rc = fchown (dirfd(dir), tmpuid, tmpgid);
 		}
 	} else {
 		rc = -1;
 	}
 
+	(void) closedir (dir);
+
 	return rc;
 }
 
+/*
+ * chown_tree - change ownership of files in a directory tree
+ *
+ *	chown_dir() walks a directory tree and changes the ownership
+ *	of all files owned by the provided user ID.
+ *
+ *	Only files owned (resp. group-owned) by old_uid (resp. by old_gid)
+ *	will have their ownership (resp. group-ownership) modified, unless
+ *	old_uid (resp. old_gid) is set to -1.
+ *
+ *	new_uid and new_gid can be set to -1 to indicate that no owner or
+ *	group-owner shall be changed.
+ */
+int chown_tree (const char *root,
+                uid_t old_uid,
+                uid_t new_uid,
+                gid_t old_gid,
+                gid_t new_gid)
+{
+	return chown_tree_at (AT_FDCWD, root, old_uid, new_uid, old_gid, new_gid);
+}