Skip to content

Commit e9ae247

Browse files
cgzoneshallyn
authored andcommitted
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.
1 parent 4b3dde0 commit e9ae247

File tree

1 file changed

+49
-81
lines changed

1 file changed

+49
-81
lines changed

libmisc/chowndir.c

Lines changed: 49 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -17,45 +17,28 @@
1717
#include "defines.h"
1818
#include <fcntl.h>
1919
#include <stdio.h>
20-
/*
21-
* chown_tree - change ownership of files in a directory tree
22-
*
23-
* chown_dir() walks a directory tree and changes the ownership
24-
* of all files owned by the provided user ID.
25-
*
26-
* Only files owned (resp. group-owned) by old_uid (resp. by old_gid)
27-
* will have their ownership (resp. group-ownership) modified, unless
28-
* old_uid (resp. old_gid) is set to -1.
29-
*
30-
* new_uid and new_gid can be set to -1 to indicate that no owner or
31-
* group-owner shall be changed.
32-
*/
33-
int chown_tree (const char *root,
20+
#include <unistd.h>
21+
22+
static int chown_tree_at (int at_fd,
23+
const char *path,
3424
uid_t old_uid,
3525
uid_t new_uid,
3626
gid_t old_gid,
3727
gid_t new_gid)
3828
{
39-
char *new_name;
40-
size_t new_name_len;
41-
int rc = 0;
42-
struct dirent *ent;
43-
struct stat sb;
4429
DIR *dir;
30+
const struct dirent *ent;
31+
struct stat dir_sb;
32+
int dir_fd, rc = 0;
4533

46-
new_name = malloc (1024);
47-
if (NULL == new_name) {
34+
dir_fd = openat (at_fd, path, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC);
35+
if (dir_fd < 0) {
4836
return -1;
4937
}
50-
new_name_len = 1024;
5138

52-
/*
53-
* Make certain the directory exists. This routine is called
54-
* directly by the invoker, or recursively.
55-
*/
56-
57-
if (access (root, F_OK) != 0) {
58-
free (new_name);
39+
dir = fdopendir (dir_fd);
40+
if (!dir) {
41+
(void) close (dir_fd);
5942
return -1;
6043
}
6144

@@ -65,68 +48,34 @@ int chown_tree (const char *root,
6548
* recursively. If not, it is checked to see if an ownership
6649
* shall be changed.
6750
*/
68-
69-
dir = opendir (root);
70-
if (NULL == dir) {
71-
free (new_name);
72-
return -1;
73-
}
74-
7551
while ((ent = readdir (dir))) {
76-
size_t ent_name_len;
7752
uid_t tmpuid = (uid_t) -1;
7853
gid_t tmpgid = (gid_t) -1;
54+
struct stat ent_sb;
7955

8056
/*
8157
* Skip the "." and ".." entries
8258
*/
83-
8459
if ( (strcmp (ent->d_name, ".") == 0)
8560
|| (strcmp (ent->d_name, "..") == 0)) {
8661
continue;
8762
}
8863

89-
/*
90-
* Make the filename for both the source and the
91-
* destination files.
92-
*/
93-
94-
ent_name_len = strlen (root) + strlen (ent->d_name) + 2;
95-
if (ent_name_len > new_name_len) {
96-
/*@only@*/char *tmp = realloc (new_name, ent_name_len);
97-
if (NULL == tmp) {
98-
rc = -1;
99-
break;
100-
}
101-
new_name = tmp;
102-
new_name_len = ent_name_len;
103-
}
104-
105-
(void) snprintf (new_name, new_name_len, "%s/%s", root, ent->d_name);
106-
107-
/* Don't follow symbolic links! */
108-
if (LSTAT (new_name, &sb) == -1) {
109-
continue;
64+
rc = fstatat (dirfd(dir), ent->d_name, &ent_sb, AT_SYMLINK_NOFOLLOW);
65+
if (rc < 0) {
66+
break;
11067
}
11168

112-
if (S_ISDIR (sb.st_mode) && !S_ISLNK (sb.st_mode)) {
113-
69+
if (S_ISDIR (ent_sb.st_mode)) {
11470
/*
11571
* Do the entire subdirectory.
11672
*/
117-
118-
rc = chown_tree (new_name, old_uid, new_uid,
119-
old_gid, new_gid);
73+
rc = chown_tree_at (dirfd(dir), ent->d_name, old_uid, new_uid, old_gid, new_gid);
12074
if (0 != rc) {
12175
break;
12276
}
12377
}
124-
#ifndef HAVE_LCHOWN
125-
/* don't use chown (follows symbolic links!) */
126-
if (S_ISLNK (sb.st_mode)) {
127-
continue;
128-
}
129-
#endif
78+
13079
/*
13180
* By default, the IDs are not changed (-1).
13281
*
@@ -136,43 +85,62 @@ int chown_tree (const char *root,
13685
* If the file is not group-owned by the group, the
13786
* group-owner is not changed.
13887
*/
139-
if (((uid_t) -1 == old_uid) || (sb.st_uid == old_uid)) {
88+
if (((uid_t) -1 == old_uid) || (ent_sb.st_uid == old_uid)) {
14089
tmpuid = new_uid;
14190
}
142-
if (((gid_t) -1 == old_gid) || (sb.st_gid == old_gid)) {
91+
if (((gid_t) -1 == old_gid) || (ent_sb.st_gid == old_gid)) {
14392
tmpgid = new_gid;
14493
}
14594
if (((uid_t) -1 != tmpuid) || ((gid_t) -1 != tmpgid)) {
146-
rc = LCHOWN (new_name, tmpuid, tmpgid);
95+
rc = fchownat (dirfd(dir), ent->d_name, tmpuid, tmpgid, AT_SYMLINK_NOFOLLOW);
14796
if (0 != rc) {
14897
break;
14998
}
15099
}
151100
}
152101

153-
free (new_name);
154-
(void) closedir (dir);
155-
156102
/*
157103
* Now do the root of the tree
158104
*/
159-
160-
if ((0 == rc) && (stat (root, &sb) == 0)) {
105+
if ((0 == rc) && (fstat (dirfd(dir), &dir_sb) == 0)) {
161106
uid_t tmpuid = (uid_t) -1;
162107
gid_t tmpgid = (gid_t) -1;
163-
if (((uid_t) -1 == old_uid) || (sb.st_uid == old_uid)) {
108+
if (((uid_t) -1 == old_uid) || (dir_sb.st_uid == old_uid)) {
164109
tmpuid = new_uid;
165110
}
166-
if (((gid_t) -1 == old_gid) || (sb.st_gid == old_gid)) {
111+
if (((gid_t) -1 == old_gid) || (dir_sb.st_gid == old_gid)) {
167112
tmpgid = new_gid;
168113
}
169114
if (((uid_t) -1 != tmpuid) || ((gid_t) -1 != tmpgid)) {
170-
rc = LCHOWN (root, tmpuid, tmpgid);
115+
rc = fchown (dirfd(dir), tmpuid, tmpgid);
171116
}
172117
} else {
173118
rc = -1;
174119
}
175120

121+
(void) closedir (dir);
122+
176123
return rc;
177124
}
178125

126+
/*
127+
* chown_tree - change ownership of files in a directory tree
128+
*
129+
* chown_dir() walks a directory tree and changes the ownership
130+
* of all files owned by the provided user ID.
131+
*
132+
* Only files owned (resp. group-owned) by old_uid (resp. by old_gid)
133+
* will have their ownership (resp. group-ownership) modified, unless
134+
* old_uid (resp. old_gid) is set to -1.
135+
*
136+
* new_uid and new_gid can be set to -1 to indicate that no owner or
137+
* group-owner shall be changed.
138+
*/
139+
int chown_tree (const char *root,
140+
uid_t old_uid,
141+
uid_t new_uid,
142+
gid_t old_gid,
143+
gid_t new_gid)
144+
{
145+
return chown_tree_at (AT_FDCWD, root, old_uid, new_uid, old_gid, new_gid);
146+
}

0 commit comments

Comments
 (0)