229 lines
6.0 KiB
Diff
229 lines
6.0 KiB
Diff
From e9ae247cb14f977d8881f481488843b10665dba8 Mon Sep 17 00:00:00 2001
|
|
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
|
|
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.
|
|
|
|
Conflict: NA
|
|
Reference: https://github.com/shadow-maint/shadow/commit/e9ae247cb14f977d8881f481488843b10665dba8
|
|
|
|
---
|
|
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 <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 DIRECT *ent;
|
|
- struct stat sb;
|
|
DIR *dir;
|
|
+ const struct DIRECT *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);
|
|
+}
|