aboutsummaryrefslogtreecommitdiffstats
path: root/main/openrc/CVE-2018-21269.patch
blob: 9975d7bf81b4ebb3824302e9bf4378d2d57310f4 (plain) (blame)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
From 577f00abe5f8ec6da40ac79d77df3e514593090d Mon Sep 17 00:00:00 2001
From: William Hubbs <w.d.hubbs@gmail.com>
Date: Wed, 11 Nov 2020 10:28:50 -0600
Subject: [PATCH] checkpath: fix CVE-2018-21269

This walks the directory path to the file we are going to manipulate to make
sure that when we create the file and change the ownership and permissions
we are working on the same file.
Also, all non-terminal symbolic links must be owned by root. This will
keep a non-root user from making a symbolic link as described in the
bug. If root creates the symbolic link, it is assumed to be trusted.

On non-linux platforms, we no longer follow non-terminal symbolic links
by default. If you need to do that, add the -s option on the checkpath
command line, but keep in mind that this is not secure.

This fixes #201.
---
 man/openrc-run.8   |   6 +++
 src/rc/checkpath.c | 103 ++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 102 insertions(+), 7 deletions(-)

diff --git a/man/openrc-run.8 b/man/openrc-run.8
index 1102daaa..ec4b88de 100644
--- a/man/openrc-run.8
+++ b/man/openrc-run.8
@@ -461,6 +461,7 @@ Mark the service as inactive.
 .Op Fl p , -pipe
 .Op Fl m , -mode Ar mode
 .Op Fl o , -owner Ar owner
+.Op Fl s , -symlinks
 .Op Fl W , -writable
 .Op Fl q , -quiet
 .Ar path ...
@@ -481,6 +482,11 @@ or with names, and are separated by a colon.
 The truncate options (-D and -F) cause the directory or file to be
 cleared of all contents.
 .Pp
+If -s is not specified on a non-linux platform, checkpath will refuse to
+allow non-terminal symbolic links to exist in the path. This is for
+security reasons so that a non-root user can't create a symbolic link to
+a root-owned file and take ownership of that file.
+.Pp
 If -W is specified, checkpath checks to see if the first path given on
 the command line is writable.  This is different from how the test
 command in the shell works, because it also checks to make sure the file
diff --git a/src/rc/checkpath.c b/src/rc/checkpath.c
index 448c9cf8..ff54a892 100644
--- a/src/rc/checkpath.c
+++ b/src/rc/checkpath.c
@@ -16,6 +16,7 @@
  *    except according to the terms contained in the LICENSE file.
  */
 
+#define _GNU_SOURCE
 #include <sys/types.h>
 #include <sys/stat.h>
 
@@ -23,6 +24,7 @@
 #include <fcntl.h>
 #include <getopt.h>
 #include <grp.h>
+#include <libgen.h>
 #include <pwd.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -44,7 +46,7 @@ typedef enum {
 
 const char *applet = NULL;
 const char *extraopts ="path1 [path2] [...]";
-const char *getoptstring = "dDfFpm:o:W" getoptstring_COMMON;
+const char *getoptstring = "dDfFpm:o:sW" getoptstring_COMMON;
 const struct option longopts[] = {
 	{ "directory",          0, NULL, 'd'},
 	{ "directory-truncate", 0, NULL, 'D'},
@@ -53,6 +55,7 @@ const struct option longopts[] = {
 	{ "pipe",               0, NULL, 'p'},
 	{ "mode",               1, NULL, 'm'},
 	{ "owner",              1, NULL, 'o'},
+	{ "symlinks",           0, NULL, 's'},
 	{ "writable",           0, NULL, 'W'},
 	longopts_COMMON
 };
@@ -64,15 +67,92 @@ const char * const longopts_help[] = {
 	"Create a named pipe (FIFO) if not exists",
 	"Mode to check",
 	"Owner to check (user:group)",
+	"follow symbolic links (irrelivent on linux)",
 	"Check whether the path is writable or not",
 	longopts_help_COMMON
 };
 const char *usagestring = NULL;
 
+static int get_dirfd(char *path, bool symlinks) {
+	char *ch;
+	char *item;
+	char *linkpath = NULL;
+	char *path_dupe;
+	char *str;
+	int components = 0;
+	int dirfd;
+	int flags = 0;
+	int new_dirfd;
+	struct stat st;
+	ssize_t linksize;
+
+	if (!path || *path != '/')
+		eerrorx("%s: empty or relative path", applet);
+	dirfd = openat(dirfd, "/", O_RDONLY);
+	if (dirfd == -1)
+		eerrorx("%s: unable to open the root directory: %s",
+				applet, strerror(errno));
+	path_dupe = xstrdup(path);
+	ch = path_dupe;
+	while (*ch) {
+		if (*ch == '/')
+			components++;
+		ch++;
+	}
+	item = strtok(path_dupe, "/");
+#ifdef O_PATH
+	flags |= O_PATH;
+#endif
+	if (!symlinks)
+		flags |= O_NOFOLLOW;
+	flags |= O_RDONLY;
+	while (dirfd > 0 && item && components > 1) {
+		str = xstrdup(linkpath ? linkpath : item);
+		new_dirfd = openat(dirfd, str, flags);
+		if (new_dirfd == -1)
+			eerrorx("%s: %s: could not open %s: %s", applet, path, str,
+					strerror(errno));
+		if (fstat(new_dirfd, &st) == -1)
+			eerrorx("%s: %s: unable to stat %s: %s", applet, path, item,
+					strerror(errno));
+		if (S_ISLNK(st.st_mode) ) {
+			if (st.st_uid != 0)
+				eerrorx("%s: %s: synbolic link %s not owned by root",
+						applet, path, str);
+			linksize = st.st_size+1;
+			if (linkpath)
+				free(linkpath);
+			linkpath = xmalloc(linksize);
+			memset(linkpath, 0, linksize);
+			if (readlinkat(new_dirfd, "", linkpath, linksize) != st.st_size)
+				eerrorx("%s: symbolic link destination changed", applet);
+			/*
+			 * now follow the symlink.
+			 */
+			close(new_dirfd);
+		} else {
+			close(dirfd);
+			dirfd = new_dirfd;
+			free(linkpath);
+			linkpath = NULL;
+			item = strtok(NULL, "/");
+			components--;
+		}
+	}
+	free(path_dupe);
+	if (linkpath) {
+		free(linkpath);
+		linkpath = NULL;
+	}
+	return dirfd;
+}
+
 static int do_check(char *path, uid_t uid, gid_t gid, mode_t mode,
-	inode_t type, bool trunc, bool chowner, bool selinux_on)
+	inode_t type, bool trunc, bool chowner, bool symlinks, bool selinux_on)
 {
 	struct stat st;
+	char *name = NULL;
+	int dirfd;
 	int fd;
 	int flags;
 	int r;
@@ -93,14 +173,16 @@ static int do_check(char *path, uid_t uid, gid_t gid, mode_t mode,
 #endif
 	if (trunc)
 		flags |= O_TRUNC;
-	readfd = open(path, readflags);
+	xasprintf(&name, "%s", basename_c(path));
+	dirfd = get_dirfd(path, symlinks);
+	readfd = openat(dirfd, name, readflags);
 	if (readfd == -1 || (type == inode_file && trunc)) {
 		if (type == inode_file) {
 			einfo("%s: creating file", path);
 			if (!mode) /* 664 */
 				mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH;
 			u = umask(0);
-			fd = open(path, flags, mode);
+			fd = openat(dirfd, name, flags, mode);
 			umask(u);
 			if (fd == -1) {
 				eerror("%s: open: %s", applet, strerror(errno));
@@ -122,7 +204,7 @@ static int do_check(char *path, uid_t uid, gid_t gid, mode_t mode,
 				    strerror (errno));
 				return -1;
 			}
-			readfd = open(path, readflags);
+			readfd = openat(dirfd, name, readflags);
 			if (readfd == -1) {
 				eerror("%s: unable to open directory: %s", applet,
 						strerror(errno));
@@ -140,7 +222,7 @@ static int do_check(char *path, uid_t uid, gid_t gid, mode_t mode,
 				    strerror (errno));
 				return -1;
 			}
-			readfd = open(path, readflags);
+			readfd = openat(dirfd, name, readflags);
 			if (readfd == -1) {
 				eerror("%s: unable to open fifo: %s", applet,
 						strerror(errno));
@@ -259,6 +341,7 @@ int main(int argc, char **argv)
 	int retval = EXIT_SUCCESS;
 	bool trunc = false;
 	bool chowner = false;
+	bool symlinks = false;
 	bool writable = false;
 	bool selinux_on = false;
 
@@ -293,6 +376,11 @@ int main(int argc, char **argv)
 				eerrorx("%s: owner `%s' not found",
 				    applet, optarg);
 			break;
+		case 's':
+#ifndef O_PATH
+			symlinks = true;
+#endif
+			break;
 		case 'W':
 			writable = true;
 			break;
@@ -320,7 +408,8 @@ int main(int argc, char **argv)
 	while (optind < argc) {
 		if (writable)
 			exit(!is_writable(argv[optind]));
-		if (do_check(argv[optind], uid, gid, mode, type, trunc, chowner, selinux_on))
+		if (do_check(argv[optind], uid, gid, mode, type, trunc, chowner,
+					symlinks, selinux_on))
 			retval = EXIT_FAILURE;
 		optind++;
 	}