From 020bf88fd1c5bdac8fc671b37c7118f5378c7047 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Wed, 12 Dec 2012 19:02:33 +0100 Subject: TOOLS: Use openat/unlinkat when removing the homedir The removal of a home directory is sensitive to concurrent modification of the directory tree being removed and can unlink files outside the directory tree. This security issue was assigned CVE-2013-0219 https://fedorahosted.org/sssd/ticket/1782 --- src/tools/files.c | 83 +++++++++++++++++++++++++++---------------------------- 1 file changed, 41 insertions(+), 42 deletions(-) (limited to 'src/tools') diff --git a/src/tools/files.c b/src/tools/files.c index d80e3bd9..3d7b8d42 100644 --- a/src/tools/files.c +++ b/src/tools/files.c @@ -78,8 +78,9 @@ struct copy_ctx { /* wrapper in order not to create a temporary context in * every iteration */ static int remove_tree_with_ctx(TALLOC_CTX *mem_ctx, - dev_t parent_dev, - const char *root); + int parent_fd, + const char *dir_name, + dev_t parent_dev); int remove_tree(const char *root) { @@ -91,7 +92,7 @@ int remove_tree(const char *root) return ENOMEM; } - ret = remove_tree_with_ctx(tmp_ctx, 0, root); + ret = remove_tree_with_ctx(tmp_ctx, AT_FDCWD, root, 0); talloc_free(tmp_ctx); return ret; } @@ -102,75 +103,75 @@ int remove_tree(const char *root) * reach the top level remove_tree() again */ static int remove_tree_with_ctx(TALLOC_CTX *mem_ctx, - dev_t parent_dev, - const char *root) + int parent_fd, + const char *dir_name, + dev_t parent_dev) { - char *fullpath = NULL; struct dirent *result; - struct dirent direntp; struct stat statres; DIR *rootdir = NULL; int ret, err; + int dir_fd; + + dir_fd = openat(parent_fd, dir_name, + O_RDONLY | O_CLOEXEC | O_DIRECTORY | O_NOFOLLOW); + if (dir_fd == -1) { + ret = errno; + DEBUG(SSSDBG_CRIT_FAILURE, ("Cannot open %s: [%d]: %s\n", + dir_name, ret, strerror(ret))); + return ret; + } - rootdir = opendir(root); + rootdir = fdopendir(dir_fd); if (rootdir == NULL) { ret = errno; - DEBUG(1, ("Cannot open directory %s [%d][%s]\n", - root, ret, strerror(ret))); + DEBUG(SSSDBG_CRIT_FAILURE, + ("Cannot open directory: [%d][%s]\n", ret, strerror(ret))); + close(dir_fd); goto fail; } - while (readdir_r(rootdir, &direntp, &result) == 0) { - if (result == NULL) { - /* End of directory */ - break; - } - - if (strcmp (direntp.d_name, ".") == 0 || - strcmp (direntp.d_name, "..") == 0) { + while ((result = readdir(rootdir)) != NULL) { + if (strcmp(result->d_name, ".") == 0 || + strcmp(result->d_name, "..") == 0) { continue; } - fullpath = talloc_asprintf(mem_ctx, "%s/%s", root, direntp.d_name); - if (fullpath == NULL) { - ret = ENOMEM; - goto fail; - } - - ret = lstat(fullpath, &statres); + ret = fstatat(dir_fd, result->d_name, + &statres, AT_SYMLINK_NOFOLLOW); if (ret != 0) { ret = errno; - DEBUG(1, ("Cannot stat %s: [%d][%s]\n", - fullpath, ret, strerror(ret))); + DEBUG(SSSDBG_CRIT_FAILURE, + ("stat failed: [%d][%s]\n", ret, strerror(ret))); goto fail; } if (S_ISDIR(statres.st_mode)) { /* if directory, recursively descend, but check if on the same FS */ if (parent_dev && parent_dev != statres.st_dev) { - DEBUG(1, ("Directory %s is on different filesystem, " - "will not follow\n", fullpath)); + DEBUG(SSSDBG_CRIT_FAILURE, + ("Directory %s is on different filesystem, " + "will not follow\n")); ret = EFAULT; goto fail; } - ret = remove_tree_with_ctx(mem_ctx, statres.st_dev, fullpath); + ret = remove_tree_with_ctx(mem_ctx, dir_fd, result->d_name, statres.st_dev); if (ret != EOK) { - DEBUG(1, ("Removing subdirectory %s failed: [%d][%s]\n", - fullpath, ret, strerror(ret))); + DEBUG(SSSDBG_CRIT_FAILURE, + ("Removing subdirectory failed: [%d][%s]\n", + ret, strerror(ret))); goto fail; } } else { - ret = unlink(fullpath); + ret = unlinkat(dir_fd, result->d_name, 0); if (ret != 0) { ret = errno; - DEBUG(1, ("Removing file %s failed: [%d][%s]\n", - fullpath, ret, strerror(ret))); + DEBUG(SSSDBG_CRIT_FAILURE, + ("Removing file failed: [%d][%s]\n", ret, strerror(ret))); goto fail; } } - - talloc_free(fullpath); } ret = closedir(rootdir); @@ -180,19 +181,17 @@ static int remove_tree_with_ctx(TALLOC_CTX *mem_ctx, goto fail; } - ret = rmdir(root); - if (ret != 0) { + ret = unlinkat(parent_fd, dir_name, AT_REMOVEDIR); + if (ret == -1) { ret = errno; - goto fail; } ret = EOK; - fail: if (rootdir) { /* clean up on abnormal exit but retain return code */ err = closedir(rootdir); if (err) { - DEBUG(1, ("closedir failed, bad dirp?\n")); + DEBUG(SSSDBG_CRIT_FAILURE, ("closedir failed, bad dirp?\n")); } } return ret; -- cgit