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
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
| | Fix text rendering in QtWebEngine:
https://issues.guix.gnu.org/52672
Patch copied from upstream:
https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/374232
From be0320febb694d850b76396346ef7ba5b45b7f0d Mon Sep 17 00:00:00 2001
From: Allan Sandfeld Jensen <allan.jensen@qt.io>
Date: Thu, 16 Sep 2021 13:17:58 +0200
Subject: [PATCH] [Backport] Linux sandbox: fix fstatat() crash
This is a reland of https://crrev.com/c/2801873.
Glibc has started rewriting fstat(fd, stat_buf) to
fstatat(fd, "", stat_buf, AT_EMPTY_PATH). This works because when
AT_EMPTY_PATH is specified, and the second argument is an empty string,
then fstatat just performs an fstat on fd like normal.
Unfortunately, fstatat() also allows stat-ing arbitrary pathnames like
with fstatat(AT_FDCWD, "/i/am/a/file", stat_buf, 0);
The baseline policy needs to prevent this usage of fstatat() since it
doesn't allow access to arbitrary pathnames.
Sadly, if the second argument is not an empty string, AT_EMPTY_PATH is
simply ignored by current kernels.
This means fstatat() is completely unsandboxable with seccomp, since
we *need* to verify that the second argument is the empty string, but
we can't dereference pointers in seccomp (due to limitations of BPF,
and the difficulty of addressing these limitations due to TOCTOU
issues).
So, this CL Traps (raises a SIGSYS via seccomp) on any fstatat syscall.
The signal handler, which runs in the sandboxed process, checks for
AT_EMPTY_PATH and the empty string, and then rewrites any applicable
fstatat() back into the old-style fstat().
Bug: 1164975
Change-Id: I3df6c04c0d781eb1f181d707ccaaead779337291
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Matthew Denton <mpdenton@chromium.org>
Cr-Commit-Position: refs/heads/master@{#903873}
Reviewed-by: Peter Varga <pvarga@inf.u-szeged.hu>
---
diff --git a/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc b/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc
index 3c67b12..ca19290 100644
--- a/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc
+++ b/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc
@@ -20,6 +20,7 @@
#include "sandbox/linux/seccomp-bpf-helpers/syscall_sets.h"
#include "sandbox/linux/seccomp-bpf/sandbox_bpf.h"
#include "sandbox/linux/services/syscall_wrappers.h"
+#include "sandbox/linux/system_headers/linux_stat.h"
#include "sandbox/linux/system_headers/linux_syscalls.h"
#if !defined(SO_PEEK_OFF)
@@ -257,6 +258,13 @@
return RestrictKillTarget(current_pid, sysno);
}
+ // The fstatat syscalls are file system syscalls, which will be denied below
+ // with fs_denied_errno. However some allowed fstat syscalls are rewritten by
+ // libc implementations to fstatat syscalls, and we need to rewrite them back.
+ if (sysno == __NR_fstatat_default) {
+ return RewriteFstatatSIGSYS(fs_denied_errno);
+ }
+
if (SyscallSets::IsFileSystem(sysno) ||
SyscallSets::IsCurrentDirectory(sysno)) {
return Error(fs_denied_errno);
diff --git a/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc b/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc
index 64ec1ce..814b700 100644
--- a/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc
+++ b/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc
@@ -50,7 +50,8 @@
namespace {
-// This also tests that read(), write() and fstat() are allowed.
+// This also tests that read(), write(), fstat(), and fstatat(.., "", ..,
+// AT_EMPTY_PATH) are allowed.
void TestPipeOrSocketPair(base::ScopedFD read_end, base::ScopedFD write_end) {
BPF_ASSERT_LE(0, read_end.get());
BPF_ASSERT_LE(0, write_end.get());
@@ -59,6 +60,20 @@
BPF_ASSERT_EQ(0, sys_ret);
BPF_ASSERT(S_ISFIFO(stat_buf.st_mode) || S_ISSOCK(stat_buf.st_mode));
+ sys_ret = fstatat(read_end.get(), "", &stat_buf, AT_EMPTY_PATH);
+ BPF_ASSERT_EQ(0, sys_ret);
+ BPF_ASSERT(S_ISFIFO(stat_buf.st_mode) || S_ISSOCK(stat_buf.st_mode));
+
+ // Make sure fstatat with anything other than an empty string is denied.
+ sys_ret = fstatat(read_end.get(), "/", &stat_buf, AT_EMPTY_PATH);
+ BPF_ASSERT_EQ(sys_ret, -1);
+ BPF_ASSERT_EQ(EPERM, errno);
+
+ // Make sure fstatat without AT_EMPTY_PATH is denied.
+ sys_ret = fstatat(read_end.get(), "", &stat_buf, 0);
+ BPF_ASSERT_EQ(sys_ret, -1);
+ BPF_ASSERT_EQ(EPERM, errno);
+
const ssize_t kTestTransferSize = 4;
static const char kTestString[kTestTransferSize] = {'T', 'E', 'S', 'T'};
ssize_t transfered = 0;
diff --git a/chromium/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc b/chromium/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc
index 76eb324..13e7180 100644
--- a/chromium/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc
+++ b/chromium/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc
@@ -6,6 +6,7 @@
#include "sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.h"
+#include <fcntl.h>
#include <stddef.h>
#include <stdint.h>
#include <string.h>
@@ -22,6 +23,7 @@
#include "sandbox/linux/seccomp-bpf/syscall.h"
#include "sandbox/linux/services/syscall_wrappers.h"
#include "sandbox/linux/system_headers/linux_seccomp.h"
+#include "sandbox/linux/system_headers/linux_stat.h"
#include "sandbox/linux/system_headers/linux_syscalls.h"
#if defined(__mips__)
@@ -355,6 +357,24 @@
return -ENOSYS;
}
+intptr_t SIGSYSFstatatHandler(const struct arch_seccomp_data& args,
+ void* fs_denied_errno) {
+ if (args.nr == __NR_fstatat_default) {
+ if (*reinterpret_cast<const char*>(args.args[1]) == '\0' &&
+ args.args[3] == static_cast<uint64_t>(AT_EMPTY_PATH)) {
+ return syscall(__NR_fstat_default, static_cast<int>(args.args[0]),
+ reinterpret_cast<default_stat_struct*>(args.args[2]));
+ }
+ return -reinterpret_cast<intptr_t>(fs_denied_errno);
+ }
+
+ CrashSIGSYS_Handler(args, fs_denied_errno);
+
+ // Should never be reached.
+ RAW_CHECK(false);
+ return -ENOSYS;
+}
+
bpf_dsl::ResultExpr CrashSIGSYS() {
return bpf_dsl::Trap(CrashSIGSYS_Handler, NULL);
}
@@ -387,6 +407,11 @@
return bpf_dsl::Trap(SIGSYSSchedHandler, NULL);
}
+bpf_dsl::ResultExpr RewriteFstatatSIGSYS(int fs_denied_errno) {
+ return bpf_dsl::Trap(SIGSYSFstatatHandler,
+ reinterpret_cast<void*>(fs_denied_errno));
+}
+
void AllocateCrashKeys() {
#if !defined(OS_NACL_NONSFI)
if (seccomp_crash_key)
diff --git a/chromium/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.h b/chromium/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.h
index 7a958b9..8cd735c 100644
--- a/chromium/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.h
+++ b/chromium/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.h
@@ -62,6 +62,19 @@
// sched_setparam(), sched_setscheduler()
SANDBOX_EXPORT intptr_t SIGSYSSchedHandler(const arch_seccomp_data& args,
void* aux);
+// If the fstatat() syscall is functionally equivalent to an fstat() syscall,
+// then rewrite the syscall to the equivalent fstat() syscall which can be
+// adequately sandboxed.
+// If the fstatat() is not functionally equivalent to an fstat() syscall, we
+// fail with -fs_denied_errno.
+// If the syscall is not an fstatat() at all, crash in the same way as
+// CrashSIGSYS_Handler.
+// This is necessary because glibc and musl have started rewriting fstat(fd,
+// stat_buf) as fstatat(fd, "", stat_buf, AT_EMPTY_PATH). We rewrite the latter
+// back to the former, which is actually sandboxable.
+SANDBOX_EXPORT intptr_t
+SIGSYSFstatatHandler(const struct arch_seccomp_data& args,
+ void* fs_denied_errno);
// Variants of the above functions for use with bpf_dsl.
SANDBOX_EXPORT bpf_dsl::ResultExpr CrashSIGSYS();
@@ -72,6 +85,7 @@
SANDBOX_EXPORT bpf_dsl::ResultExpr CrashSIGSYSFutex();
SANDBOX_EXPORT bpf_dsl::ResultExpr CrashSIGSYSPtrace();
SANDBOX_EXPORT bpf_dsl::ResultExpr RewriteSchedSIGSYS();
+SANDBOX_EXPORT bpf_dsl::ResultExpr RewriteFstatatSIGSYS(int fs_denied_errno);
// Allocates a crash key so that Seccomp information can be recorded.
void AllocateCrashKeys();
diff --git a/chromium/sandbox/linux/syscall_broker/broker_process.cc b/chromium/sandbox/linux/syscall_broker/broker_process.cc
index d72c9d2..36df5e4 100644
--- a/chromium/sandbox/linux/syscall_broker/broker_process.cc
+++ b/chromium/sandbox/linux/syscall_broker/broker_process.cc
@@ -122,44 +122,49 @@
}
bool BrokerProcess::IsSyscallBrokerable(int sysno, bool fast_check) const {
+ // The syscalls unavailable on aarch64 are all blocked by Android's default
+ // seccomp policy, even on non-aarch64 architectures. I.e., the syscalls XX()
+ // with a corresponding XXat() versions are typically unavailable in aarch64
+ // and are default disabled in Android. So, we should refuse to broker them
+ // to be consistent with the platform's restrictions.
switch (sysno) {
-#if !defined(__aarch64__)
+#if !defined(__aarch64__) && !defined(OS_ANDROID)
case __NR_access:
#endif
case __NR_faccessat:
return !fast_check || allowed_command_set_.test(COMMAND_ACCESS);
-#if !defined(__aarch64__)
+#if !defined(__aarch64__) && !defined(OS_ANDROID)
case __NR_mkdir:
#endif
case __NR_mkdirat:
return !fast_check || allowed_command_set_.test(COMMAND_MKDIR);
-#if !defined(__aarch64__)
+#if !defined(__aarch64__) && !defined(OS_ANDROID)
case __NR_open:
#endif
case __NR_openat:
return !fast_check || allowed_command_set_.test(COMMAND_OPEN);
-#if !defined(__aarch64__)
+#if !defined(__aarch64__) && !defined(OS_ANDROID)
case __NR_readlink:
#endif
case __NR_readlinkat:
return !fast_check || allowed_command_set_.test(COMMAND_READLINK);
-#if !defined(__aarch64__)
+#if !defined(__aarch64__) && !defined(OS_ANDROID)
case __NR_rename:
#endif
case __NR_renameat:
case __NR_renameat2:
return !fast_check || allowed_command_set_.test(COMMAND_RENAME);
-#if !defined(__aarch64__)
+#if !defined(__aarch64__) && !defined(OS_ANDROID)
case __NR_rmdir:
return !fast_check || allowed_command_set_.test(COMMAND_RMDIR);
#endif
-#if !defined(__aarch64__)
+#if !defined(__aarch64__) && !defined(OS_ANDROID)
case __NR_stat:
case __NR_lstat:
#endif
@@ -184,7 +189,7 @@
return !fast_check || allowed_command_set_.test(COMMAND_STAT);
#endif
-#if !defined(__aarch64__)
+#if !defined(__aarch64__) && !defined(OS_ANDROID)
case __NR_unlink:
return !fast_check || allowed_command_set_.test(COMMAND_UNLINK);
#endif
diff --git a/chromium/sandbox/linux/syscall_broker/broker_process_unittest.cc b/chromium/sandbox/linux/syscall_broker/broker_process_unittest.cc
index b1d7106..15e00d5 100644
--- a/chromium/sandbox/linux/syscall_broker/broker_process_unittest.cc
+++ b/chromium/sandbox/linux/syscall_broker/broker_process_unittest.cc
@@ -1596,52 +1596,52 @@
const base::flat_map<BrokerCommand, base::flat_set<int>> kSysnosForCommand = {
{COMMAND_ACCESS,
{__NR_faccessat,
-#if defined(__NR_access)
+#if defined(__NR_access) && !defined(OS_ANDROID)
__NR_access
#endif
}},
{COMMAND_MKDIR,
{__NR_mkdirat,
-#if defined(__NR_mkdir)
+#if defined(__NR_mkdir) && !defined(OS_ANDROID)
__NR_mkdir
#endif
}},
{COMMAND_OPEN,
{__NR_openat,
-#if defined(__NR_open)
+#if defined(__NR_open) && !defined(OS_ANDROID)
__NR_open
#endif
}},
{COMMAND_READLINK,
{__NR_readlinkat,
-#if defined(__NR_readlink)
+#if defined(__NR_readlink) && !defined(OS_ANDROID)
__NR_readlink
#endif
}},
{COMMAND_RENAME,
{__NR_renameat,
-#if defined(__NR_rename)
+#if defined(__NR_rename) && !defined(OS_ANDROID)
__NR_rename
#endif
}},
{COMMAND_UNLINK,
{__NR_unlinkat,
-#if defined(__NR_unlink)
+#if defined(__NR_unlink) && !defined(OS_ANDROID)
__NR_unlink
#endif
}},
{COMMAND_RMDIR,
{__NR_unlinkat,
-#if defined(__NR_rmdir)
+#if defined(__NR_rmdir) && !defined(OS_ANDROID)
__NR_rmdir
#endif
}},
{COMMAND_STAT,
{
-#if defined(__NR_stat)
+#if defined(__NR_stat) && !defined(OS_ANDROID)
__NR_stat,
#endif
-#if defined(__NR_lstat)
+#if defined(__NR_lstat) && !defined(OS_ANDROID)
__NR_lstat,
#endif
#if defined(__NR_fstatat)
diff --git a/chromium/sandbox/linux/system_headers/linux_stat.h b/chromium/sandbox/linux/system_headers/linux_stat.h
index 35788eb..83b89ef 100644
--- a/chromium/sandbox/linux/system_headers/linux_stat.h
+++ b/chromium/sandbox/linux/system_headers/linux_stat.h
@@ -157,6 +157,10 @@
};
#endif
+#if !defined(AT_EMPTY_PATH)
+#define AT_EMPTY_PATH 0x1000
+#endif
+
// On 32-bit systems, we default to the 64-bit stat struct like libc
// implementations do. Otherwise we default to the normal stat struct which is
// already 64-bit.
|