From e9ae247cb14f977d8881f481488843b10665dba8 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Christian=20G=C3=B6ttsche?= Date: Fri, 5 Aug 2022 17:57:19 +0200 Subject: [PATCH] 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 | 130 +++++++++++++++++---------------------------- 1 file changed, 49 insertions(+), 81 deletions(-) diff --git a/libmisc/chowndir.c b/libmisc/chowndir.c index 0edc3b609..d31618a56 100644 --- a/libmisc/chowndir.c +++ b/libmisc/chowndir.c @@ -17,45 +17,28 @@ #include "defines.h" #include #include -/* - * 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 + +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); +} -- 2.47.2