From e9ae247cb14f977d8881f481488843b10665dba8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?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 | 246 ++++++++++++++++++++------------------------- 1 file changed, 107 insertions(+), 139 deletions(-) diff --git a/libmisc/chowndir.c b/libmisc/chowndir.c index 0edc3b60..d31618a5 100644 --- a/libmisc/chowndir.c +++ b/libmisc/chowndir.c @@ -17,6 +17,112 @@ #include "defines.h" #include #include +#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) +{ + DIR *dir; + const struct dirent *ent; + struct stat dir_sb; + int dir_fd, rc = 0; + + dir_fd = openat (at_fd, path, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + if (dir_fd < 0) { + return -1; + } + + dir = fdopendir (dir_fd); + if (!dir) { + (void) close (dir_fd); + return -1; + } + + /* + * Open the directory and read each entry. Every entry is tested + * to see if it is a directory, and if so this routine is called + * recursively. If not, it is checked to see if an ownership + * shall be changed. + */ + while ((ent = readdir (dir))) { + 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; + } + + rc = fstatat (dirfd(dir), ent->d_name, &ent_sb, AT_SYMLINK_NOFOLLOW); + if (rc < 0) { + break; + } + + if (S_ISDIR (ent_sb.st_mode)) { + /* + * Do the entire subdirectory. + */ + rc = chown_tree_at (dirfd(dir), ent->d_name, old_uid, new_uid, old_gid, new_gid); + if (0 != rc) { + break; + } + } + + /* + * By default, the IDs are not changed (-1). + * + * If the file is not owned by the user, the owner is not + * changed. + * + * If the file is not group-owned by the group, the + * group-owner is not changed. + */ + if (((uid_t) -1 == old_uid) || (ent_sb.st_uid == old_uid)) { + tmpuid = new_uid; + } + 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 = fchownat (dirfd(dir), ent->d_name, tmpuid, tmpgid, AT_SYMLINK_NOFOLLOW); + if (0 != rc) { + break; + } + } + } + + /* + * Now do the root of the tree + */ + 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) || (dir_sb.st_uid == old_uid)) { + tmpuid = new_uid; + } + 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 = fchown (dirfd(dir), tmpuid, tmpgid); + } + } else { + rc = -1; + } + + (void) closedir (dir); + + return rc; +} + /* * chown_tree - change ownership of files in a directory tree * @@ -36,143 +142,5 @@ int chown_tree (const char *root, 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; - - new_name = malloc (1024); - if (NULL == new_name) { - 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); - return -1; - } - - /* - * Open the directory and read each entry. Every entry is tested - * to see if it is a directory, and if so this routine is called - * 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; - - /* - * 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; - } - - if (S_ISDIR (sb.st_mode) && !S_ISLNK (sb.st_mode)) { - - /* - * Do the entire subdirectory. - */ - - rc = chown_tree (new_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). - * - * If the file is not owned by the user, the owner is not - * changed. - * - * 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)) { - tmpuid = new_uid; - } - if (((gid_t) -1 == old_gid) || (sb.st_gid == old_gid)) { - tmpgid = new_gid; - } - if (((uid_t) -1 != tmpuid) || ((gid_t) -1 != tmpgid)) { - rc = LCHOWN (new_name, tmpuid, tmpgid); - if (0 != rc) { - break; - } - } - } - - free (new_name); - (void) closedir (dir); - - /* - * Now do the root of the tree - */ - - if ((0 == rc) && (stat (root, &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)) { - tmpuid = new_uid; - } - if (((gid_t) -1 == old_gid) || (sb.st_gid == old_gid)) { - tmpgid = new_gid; - } - if (((uid_t) -1 != tmpuid) || ((gid_t) -1 != tmpgid)) { - rc = LCHOWN (root, tmpuid, tmpgid); - } - } else { - rc = -1; - } - - return rc; + return chown_tree_at (AT_FDCWD, root, old_uid, new_uid, old_gid, new_gid); } -