[gentoo-commits] proj/sandbox:master commit in: libsandbox/, /, tests/, libsandbox/wrapper-funcs/
commit: 2f6a725d660c61230de21748effa685ee9b3cdaa Author: Mike Frysinger gentoo org> AuthorDate: Fri Oct 22 02:38:12 2021 + Commit: Mike Frysinger gentoo org> CommitDate: Fri Oct 22 02:47:38 2021 + URL:https://gitweb.gentoo.org/proj/sandbox.git/commit/?id=2f6a725d libsandbox: add renameat2 wrapper It's basically renameat at this point as we don't care about the flags. Signed-off-by: Mike Frysinger gentoo.org> configure.ac | 1 + libsandbox/libsandbox.c | 2 ++ libsandbox/symbols.h.in | 1 + libsandbox/trace.c | 2 ++ libsandbox/wrapper-funcs/renameat2.c | 11 +++ tests/Makefile.am| 1 + tests/renameat2-0.c | 22 ++ tests/renameat2-1.sh | 8 tests/renameat2.at | 3 +++ 9 files changed, 51 insertions(+) diff --git a/configure.ac b/configure.ac index f68cf90..e16892f 100644 --- a/configure.ac +++ b/configure.ac @@ -211,6 +211,7 @@ AC_CHECK_FUNCS_ONCE(m4_flatten([ realpath remove renameat + renameat2 rmdir setenv strcasecmp diff --git a/libsandbox/libsandbox.c b/libsandbox/libsandbox.c index b084daa..c00c92c 100644 --- a/libsandbox/libsandbox.c +++ b/libsandbox/libsandbox.c @@ -682,6 +682,7 @@ static bool symlink_func(int sb_nr, int flags, const char *abs_path) sb_nr == SB_NR_REMOVE || sb_nr == SB_NR_RENAME || sb_nr == SB_NR_RENAMEAT || + sb_nr == SB_NR_RENAMEAT2|| sb_nr == SB_NR_RMDIR|| sb_nr == SB_NR_SYMLINK || sb_nr == SB_NR_SYMLINKAT)) @@ -795,6 +796,7 @@ static int check_access(sbcontext_t *sbcontext, int sb_nr, const char *func, sb_nr == SB_NR_REMOVE || sb_nr == SB_NR_RENAME || sb_nr == SB_NR_RENAMEAT|| + sb_nr == SB_NR_RENAMEAT2 || sb_nr == SB_NR_RMDIR || sb_nr == SB_NR_SYMLINK || sb_nr == SB_NR_SYMLINKAT || diff --git a/libsandbox/symbols.h.in b/libsandbox/symbols.h.in index 0154c2a..954d5ae 100644 --- a/libsandbox/symbols.h.in +++ b/libsandbox/symbols.h.in @@ -34,6 +34,7 @@ faccessat remove rename renameat +renameat2 rmdir symlink symlinkat diff --git a/libsandbox/trace.c b/libsandbox/trace.c index 4d145a3..77991e1 100644 --- a/libsandbox/trace.c +++ b/libsandbox/trace.c @@ -323,6 +323,8 @@ static bool trace_check_syscall(const struct syscall_entry *se, void *regs) else if (nr == SB_NR_MKNODAT) return trace_check_syscall_DC (&state); else if (nr == SB_NR_RENAME)return trace_check_syscall_C (&state) && _trace_check_syscall_C (&state, 2); + else if (nr == SB_NR_RENAMEAT2) return trace_check_syscall_DC (&state) && + _trace_check_syscall_DC (&state, 3); else if (nr == SB_NR_RENAMEAT) return trace_check_syscall_DC (&state) && _trace_check_syscall_DC (&state, 3); else if (nr == SB_NR_RMDIR) return trace_check_syscall_C (&state); diff --git a/libsandbox/wrapper-funcs/renameat2.c b/libsandbox/wrapper-funcs/renameat2.c new file mode 100644 index 000..4a2e29b --- /dev/null +++ b/libsandbox/wrapper-funcs/renameat2.c @@ -0,0 +1,11 @@ +/* + * renameat2() wrapper. + * + * Copyright 1999-2021 Gentoo Foundation + * Licensed under the GPL-2 + */ + +#define WRAPPER_ARGS_PROTO int olddirfd, const char *oldpath, int newdirfd, const char *newpath, unsigned int flags +#define WRAPPER_ARGS olddirfd, oldpath, newdirfd, newpath, flags +#define WRAPPER_SAFE() (SB_SAFE_AT(olddirfd, oldpath, 0) && SB_SAFE_AT(newdirfd, newpath, 0)) +#include "__wrapper_simple.c" diff --git a/tests/Makefile.am b/tests/Makefile.am index c899603..e47c996 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -57,6 +57,7 @@ check_PROGRAMS = \ remove-0 \ rename-0 \ renameat-0 \ + renameat2-0 \ rmdir-0 \ signal_static-0 \ symlink-0 \ diff --git a/tests/renameat2-0.c b/tests/renameat2-0.c new file mode 100644 index 000..6041d69 --- /dev/null +++ b/tests/renameat2-0.c @@ -0,0 +1,22 @@ +#define CONFIG HAVE_RENAMEAT2 +#define FUNC renameat2 +#define SFUNC "renameat2" +#define FUNC_STR "%i, \"%s\", %i, \"%s\", %i" +#define FUNC_IMP olddirfd, oldpath, newdirfd, newpath, 0 +#define ARG_CNT 4 +#define ARG_USE "(old) (old) (new) (new)" + +#define process_args() \ + s = argv[i++]; \ + int olddirfd = at_get_fd(s); \ + \ + s = argv[i++]; \ + char *oldpath = s; \ + \ + s = argv[i++]; \ + int newdirfd = at_get_fd(s); \ + \ + s = argv[i++]; \ + char *newpath = s; + +#include "test-skel-0.c" diff --git a/tests/renameat2-1.sh b/tests/renameat2
[gentoo-commits] proj/sandbox:master commit in: libsandbox/, tests/, libsandbox/wrapper-funcs/
commit: f3e51a930312422cc78b693a247b7c5704ac90a2
Author: Sergei Trofimovich gentoo org>
AuthorDate: Sun Jan 6 09:32:55 2019 +
Commit: Sergei Trofimovich gentoo org>
CommitDate: Tue Jan 8 22:48:44 2019 +
URL:https://gitweb.gentoo.org/proj/sandbox.git/commit/?id=f3e51a93
exec*() wrappers: never mutate 'environ' of host process
In bug #669702 gcc exposed sandbox bug where execv()
wrapper changed 'environ' global variable underneath.
A few GNU projects (pex_unix_exec_child in gcc and gdb)
use the following idiom:
for (;;) {
vfork();
char ** save_environ = environ; // [1]
if (child) {
environ = child_environ; // [2]
execv(payload); // [3]
}
if (parent) {
environ = save_environ; // [4]
...
waitpid(child, ...);
}
}
Code above assumes that execv() does not mutate 'environ'.
In case of #669702 sandbox's execv() wrapper at '[3]' mutated
'environ' and relocated it (via maloc()/free() internally).
This caused '[4]' to point 'environ' fo freed location.
The change fixes it in a following way:
- execv() call now works more like execve() call by mutating
external array and substitutes 'environ' only for a period
of 'execv()' execution.
- add basic execv()/'environ' corruption test
Tested on:
- linux/glibc-2.28
- linux/uclibc-ng-1.0.31
Reported-and-tested-by: Walther
Reported-by: 0x6d6174 posteo.de
Reported-by: Andrey Korolyov
Bug: https://bugs.gentoo.org/669702
Signed-off-by: Sergei Trofimovich gentoo.org>
libsandbox/libsandbox.c | 92 +++
libsandbox/libsandbox.h | 17 +-
libsandbox/wrapper-funcs/__wrapper_exec.c | 15 +++--
tests/Makefile.am | 1 +
tests/execv-0.c | 21 +++
tests/execv-1.sh | 4 ++
tests/execv.at| 1 +
7 files changed, 96 insertions(+), 55 deletions(-)
diff --git a/libsandbox/libsandbox.c b/libsandbox/libsandbox.c
index e0c9d1a..77e9959 100644
--- a/libsandbox/libsandbox.c
+++ b/libsandbox/libsandbox.c
@@ -1122,10 +1122,18 @@ typedef struct {
*
* XXX: Might be much nicer if we could serialize these vars behind the back of
* the program. Might be hard to handle LD_PRELOAD though ...
+ *
+ * execv*() must never modify environment inplace with
+ * setenv/putenv/unsetenv as it can relocate 'environ' and break
+ * vfork()/execv() users: https://bugs.gentoo.org/669702
*/
-char **sb_check_envp(char **envp, size_t *mod_cnt, bool insert)
+struct sb_envp_ctx sb_new_envp(char **envp, bool insert)
{
- char **my_env;
+ struct sb_envp_ctx r = {
+ .sb_envp = envp,
+ .orig_envp = envp,
+ .__mod_cnt = 0,
+ };
char *entry;
size_t count, i;
env_pair vars[] = {
@@ -1152,7 +1160,7 @@ char **sb_check_envp(char **envp, size_t *mod_cnt, bool
insert)
/* If sandbox is explicitly disabled, do not propagate the vars
* and just return user's envp */
if (!sbcontext.on)
- return envp;
+ return r;
/* First figure out how many vars are already in the env */
found_var_cnt = 0;
@@ -1188,7 +1196,7 @@ char **sb_check_envp(char **envp, size_t *mod_cnt, bool
insert)
if ((insert && num_vars == found_var_cnt) ||
(!insert && found_var_cnt == 0))
/* Use the user's envp */
- return envp;
+ return r;
/* Ok, we need to create our own envp, as we need to restore stuff
* and we should not touch the user's envp. First we add our vars,
@@ -1208,61 +1216,50 @@ char **sb_check_envp(char **envp, size_t *mod_cnt, bool
insert)
vars[13].value = "1";
}
- my_env = NULL;
+ char ** my_env = NULL;
if (!insert) {
- if (mod_cnt) {
- str_list_for_each_item(envp, entry, count) {
- for (i = 0; i < num_vars; ++i)
- if (i != 12 && is_env_var(entry,
vars[i].name, vars[i].len)) {
- (*mod_cnt)++;
- goto skip;
- }
- str_list_add_item(my_env, entry, error);
- skip: ;
- }
- } else {
+ str_list_for_each_item(envp, entry, count) {
for (i = 0; i < num_vars; ++i)
- if (i != 12) unsetenv(vars[i].name);
+ if (i != 12 /* LD_LIBRARY_PATH index */
+ && is_env_var(entry, vars[i].name,
vars[i].len)) {
+ r.__mod_cnt++;
+ goto skip;
+ }
+ str_list_ad
