| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
| |
Signed-off-by: Michał Górny <mgorny@gentoo.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
All scripts assume that ran tools matck tested sandbox's ABI.
Most scripts have a guard against ABI check, but script-16 was missing it.
It's afollow-up commit to 24fd102c9976
("check_syscall(): turn internal sandbox violation into denywrite")
Reported-by: Michał Górny
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
Closes: https://bugs.gentoo.org/590084
Signed-off-by: Michał Górny <mgorny@gentoo.org>
|
|
|
|
| |
Signed-off-by: Michał Górny <mgorny@gentoo.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In #590084 test suite performed to list files in a deleted directory:
$ sandbox 'mkdir /tmp/zzz; cd /tmp/zzz; rmdir /tmp/zzz; ls'
* sandbox-2.18/libsandbox/libsandbox.c:check_syscall():974: failure (No such file or directory):
* ISE: opendir(.)
abs_path: (null)
res_path: (null)
Another reproducer is to create file outside deleted directory relative
to that directory:
$ sandbox 'mkdir /tmp/zzz; cd /tmp/zzz; rmdir /tmp/zzz; touch ../foo'
* sandbox-2.18/libsandbox/libsandbox.c:check_syscall():974: failure (No such file or directory):
* ISE: open_wr(../foo)
abs_path: (null)
res_path: (null)
sandbox can't validate safety of any of these operations as kernel does not
provide a mechanism to resolve '.' back to an absolute path.
As it's a rare condition let's turn it into a sandbox violation instead
of internal sandbox error and link to the bug with details in the error message.
Report after the change looks like:
$ ./sandbox.sh 'mkdir /tmp/zzz; cd /tmp/zzz; rmdir /tmp/zzz; touch ../foo'
* ACCESS DENIED: open_wr: '../foo' (from deleted directory, see https://bugs.gentoo.org/590084)
* ACCESS DENIED: utimensat: '../foo' (from deleted directory, see https://bugs.gentoo.org/590084)
touch: cannot touch '../foo': Permission denied
Reported-by: Mike Gilbert
Bug: https://bugs.gentoo.org/590084
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
Signed-off-by: Michał Górny <mgorny@gentoo.org>
|
|
|
|
|
| |
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
Signed-off-by: Michał Górny <mgorny@gentoo.org>
|
|
|
|
|
|
|
|
| |
The menu entry for Sandbox is not very useful, so stop installing
it by default. Leave the files in place in case somebody wanted them.
Bug: https://bugs.gentoo.org/710920
Signed-off-by: Michał Górny <mgorny@gentoo.org>
|
|
|
|
| |
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
x86_64-gentoo-linux-musl fails a single test:
83: utimensat/3 FAILED (utimensat.at:3)
The test checks if sandbox does not crash when
utimensat(<filefd>, NULL, NULL, 0)
is called. The behaviour is not specified by POSIX
but glibc returns EINVAL for such a case. Thus the
test behaves differently on varius libs.
https://www.openwall.com/lists/musl/2019/06/25/1 has
a conversation with musl upstream.
The change restricts test down to glibc targets.
Bug: https://bugs.gentoo.org/549108
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Avoid libc's symbol rename via #define. musl defines aliases as:
#define mkstemp64 mkstemp
#define mkstemps64 mkstemps
This causes libsandbox's aliases to clash with one another,
like mkstemp and mkstemp64.
The change does not break glibc and restores musl support.
Bug: https://bugs.gentoo.org/549108
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
glibc defines ptrace as:
long ptrace(enum __ptrace_request request, pid_t pid, void *addr, void *data);
musl defines ptrace as:
long ptrace(int, ...);
This causes build failure in for of:
../../sandbox-2.17/libsandbox/trace/linux/x86_64.c: In function 'trace_set_ret':
../../sandbox-2.17/libsandbox/trace/linux/x86_64.c:99:2: error: type of formal parameter 1 is incomplete
trace_set_regs(regs);
^~~~~~~~~~~~~~
Let's clobber to 'int' lowest common denominator.
Bug: https://bugs.gentoo.org/549108
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
|
|
|
|
| |
Signed-off-by: Andreas K. Hüttel <dilfridge@gentoo.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
As reported by Mike Lothian below failed as:
$ ./configure LDFLAGS=-fuse-ld=gold
> checking libc path... configure: error: Unable to determine LIBC PATH (/lib64/libc.so.6")
The regression happened in 3c0010366
("configure.ac: add lld detection support")
where "attempt" keyword in verbose linker log was used as a hint
to use bfd or gold output parser. Unfortunately ld.gold uses
"Attempt" spelling.
The change is to use the "[Aa]ttempt" substring matcher.
Tweak the parser to also match on "[Aa]ttempt" explicitly.
Tested on bfd, gold and lld.
Reported-by: Mike Lothian
Bug: https://bugs.gentoo.org/680034
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
Signed-off-by: Andreas K. Hüttel <dilfridge@gentoo.org>
|
|
|
|
| |
Signed-off-by: Andreas K. Hüttel <dilfridge@gentoo.org>
|
|
|
|
|
|
|
|
|
|
| |
With this change
$ ./configure CC=clang LDFLAGS='-Wl,--hash-style=gnu -fuse-ld=lld'
$ make check
exposes 35 test failures
Bug: https://bugs.gentoo.org/672918
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In bug #672918 exec's ELF parser was going out of symbol table
range. It happened to trigger on a binary linked by LLVM's lld.
sandbox has no precise way to determine symbol table size and
uses the heuristic of detecting SYMTAB dynamic section size:
STRTAB (or GNU_LIBLIST for prelinked binaries).
This is a weak assumption and does not hold for lld-linked
binaries where the ordering is SYMTAB->GNU_HASH->STRTAB.
As a result sandbox SIGSEGVs when parses GNU_HASH as SYMTAB
entries.
The change adds GNU_HASH parser to iterate exactly through
exported symbols. That way we avoid heuristic code completely.
While at it repobe GNU_LIBLIST hack as we should not rely
on size heuristics anymore.
This has a nice side-effect of speeding up ELF parsing as
GHU_HASH only iterates through exported symbols. HASH
comparison has all dynamic symbols: both exported and
imported.
Reported-by: Dennis Schridde
Bug: https://bugs.gentoo.org/672918
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
|
|
|
|
| |
Signed-off-by: Michał Górny <mgorny@gentoo.org>
|
|
|
|
| |
Signed-off-by: Michał Górny <mgorny@gentoo.org>
|
|
|
|
| |
Signed-off-by: Michał Górny <mgorny@gentoo.org>
|
|
|
|
| |
Signed-off-by: Michał Górny <mgorny@gentoo.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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 <slyfox@gentoo.org>
|
|
|
|
| |
Signed-off-by: Michał Górny <mgorny@gentoo.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
If /proc was mounted by a process in a different pid namespace,
getpid cannot be used create a valid /proc/<pid> path. Instead
use sb_get_fd_dir() which works in any case. This implements
option 3 of these choices:
1) Always create a mount namespace when creating a pid namespace,
and remount /proc so that /proc/<pid> entries are always consistent
with the current pid namespace.
2) Use readlink on /proc/self instead of getpid to determine the pid
of self in the pid namespace of the /proc mount.
3) Use /proc/self or /dev/fd directly.
Bug: https://bugs.gentoo.org/670966
Signed-off-by: Zac Medico <zmedico@gentoo.org>
Closes: https://github.com/gentoo/sandbox/pull/1
Signed-off-by: Michał Górny <mgorny@gentoo.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Remove '-nodefaultlibs' from linking flags for libsandbox as it is
apparently meaningless and broken at the same time. When regular
libtool is used, it silently strips the option, making it meaningless.
When slibtool is used instead, it passes the option which causes linking
to fail due to undefined symbols.
Thanks to the bug reporter and slibtool devs from researching
the problem in detail.
Bug: https://bugs.gentoo.org/657184
|
| |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Do not enforce restoring sandbox variables in the environment if sandbox
is explicitly disabled. This makes it possible to set SANDBOX_ON=0
and then unset LD_PRELOAD without having to resort to ugly hacks to
prevent sandbox from restoring itself.
The only limitation is that if user sets SANDBOX_ON=0 first, then wipes
the environment, he will no longer be able to reenable sandbox via doing
SANDBOX_ON=1. However, it is rather unlikely that such a thing would
need to happen in real use.
Bug: https://bugs.gentoo.org/592750
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Sandbox commit 55087abd8dc9802cf68cade776fe612a3f19f6a1 is for the purpose
of preventing a loop or deadlock caused by a package implementing its own
libc memory allocation functions, which themselves may call on a sandbox wrapped
system calls, whose implementation depends on further calls to such memory
functions. If any binaries export such symbols, sandbox assumes the worst
and prevents loading of libsandbox.so and instead opts for ptrace.
In preventing the loading of libsandbox, it removes all variables whose
env_pair.name field matches the name of an environment variable from the
environment, for all env_pairs of vars[] in
char **sb_check_envp(char **envp, size_t *mod_cnt, bool insert) in
"libsandbox/libsandbox.c". This includes not just the usual environment
variables prefixed with 'SANDBOX_' but also LD_PRELOAD and LD_LIBRARY_PATH.
LD_PRELOAD clearly should be removed. But LD_LIBRARY_PATH would only seem
to be trouble if used with LD_PRELOAD. As such it makes sense to me to
prevent the removal of LD_LIBRARY_PATH.
Given the fact that the the positions of the env_pairs in vars[] are intended
to be hard-coded (from libsandbox.c: /* Indices matter -- see init below */),
this commit uses the index of the env_pair corresponding to LD_LIBRARY_PATH to
prevent its removal.
|
|
|
|
|
|
| |
Fix the path matching code to match prefixes component-wide rather than
literally. This means that a path such as '/foo' will no longer match
'/foobar' but only '/foo' and its subdirectories (if it is a directory).
|
|
|
|
|
|
|
|
| |
Remove the hack supposedly responsible for making it possible to remove
symbolic links to protected files. The hack was probably necessary back
when the write check was performed on fully resolved path. However,
currently the path resolution is no longer performed when the operation
does not resolve symlinks, effectively making the hack redundant.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Add a pre-check for opendir that catches too long name arguments
given to opendir, as it would get messed up and abort before it
even gets to the open*() syscall (which would handle it correctly),
due to opendir going through before_syscall/check_syscall, even
though it isn't a true syscall and it getting cut to SB_PATH_MAX
inbetween and getting confused somewhere.
Test case added by Michał Górny <mgorny@gentoo.org>.
Bug: https://bugs.gentoo.org/553092
Signed-off-by: Mart Raudsepp <leio@gentoo.org>
|
|
|
|
|
|
|
| |
These funcs don't deref their path args, so flag them as such.
URL: https://bugs.gentoo.org/612202
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
|
| |
Since sb_maybe_gdb is set up as a stub macro, make sure we don't define
the function either to cut down on size and build failures (when the
macro tries to expand the function prototype).
URL: https://bugs.gentoo.org/600550
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When prelink runs on an ELF, it moves the string table from right
after the symbol table to the end, and then replaces the string
table with its liblist table. This ends up breaking sandbox's
assumption that the string table always follows the symbol table
leading to prelinked ELFs crashing.
Update the range check to use the liblist table when available.
Since the prelink code has this logic hardcoded (swapping the
string table for the liblist table), this should be OK for now.
URL: https://bugs.gentoo.org/599894
Reported-by: Anders Larsson <anders.gentoo@larsson.xyz>
Reported-by: Kenton Groombridge <rustyvega@comcast.net>
Reported-by: Marien Zwart <marien.zwart@gmail.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
| |
URL: https://bugs.gentoo.org/578516
Reported-by: Toralf Förster <toralf.foerster@gmx.de>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
|
|
| |
The strtab assumption works if there is no SysV hash table.
Add logic to handle that scenario.
URL: https://bugs.gentoo.org/578524
Reported-by: Toralf Förster <toralf.foerster@gmx.de>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
| |
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
|
| |
Commit 48520a35697aa39bed046b9668a3e3e5f8a8ba93 fixed the configure logic,
but the build would fail to link for x86 systems as the syscall table was
not actually set up.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
| |
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
| |
Sometimes the child process can get wedged and not respond to CTRL+C,
so add an escape hatch so the user can easily force SIGKILL.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
|
| |
Showing just the resolved paths isn't too helpful when they're both
NULL. Also include the failing func & original file path.
URL: https://bugs.gentoo.org/553092
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
If an app installs its own memory allocator by overriding the internal
glibc symbols, then we can easily hit a loop that cannot be broken: the
dlsym functions can attempt to allocate memory, and sandbox relies on
them to find the "real" functions. So when someone calls a symbol that
the sandbox protects, we call dlsym, and that calls malloc, which calls
back into the app, and their allocator might use another symbol such as
open ... which is protected by the sandbox. So we hit the loop like:
-> open -> libsandbox:open -> dlsym -> malloc -> open ->
libsandbox:open -> dlsym -> malloc -> ...
Change the exec checking logic to scan the ELF instead. If it exports
these glibc symbols, then we have to assume it can trigger a loop, so
scrub the sandbox environment to prevent us from being loaded. Then we
use the out-of-process tracer (i.e. ptrace). This should generally be
as robust anyways ... if it's not, that's a bug we want to fix as this
is the same code used for static apps.
URL: http://crbug.com/586444
Reported-by: Ryo Hashimoto <hashimoto@chromium.org>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
| |
URL: http://bugs.gentoo.org/290249
Reported-by: Diego E. Pettenò <flameeyes@gentoo.org>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
| |
In commit 7a923f646ce10b7dec3c7ae5fe2079c10aa21752, we dropped the same.h
header, but the build still listed it. Drop it from the distdir list.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
| |
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
| |
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
| |
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
| |
This verifies the error code setting with ptrace logic -- if the ptrace
code is broken, the errno will often be ENOSYS instead of EPERM.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
|
| |
We don't provide same_name because the one caller we don't use, but it
relies on gc-sections to avoid link errors. That flag doesn't work on
ia64 though, so we need to hand delete the one caller. Ugh.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
| |
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|