| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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>
|
| |
|
|
|
|
|
|
| |
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).
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
| |
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>
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
|
|
|
|
|
| |
When the target uses a func that operates on a symlink, we should not
dereference that symlink when trying to validate the call. It's both
a waste of time and it subtly breaks code that checks atime updates.
The act of reading symlinks is enough to cause their atime to change.
URL: https://bugs.gentoo.org/415475
Reported-by: Marien Zwart <marienz@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>
|
|
|
|
|
|
|
|
|
|
|
| |
If the target passes a bad pointer to the kernel, then trying to extract
the data via ptrace will also throw an error. The tracing code should not
abort though as there's no valid address to check, and kernel itself will
return an error for us. Simply return and move on.
URL: https://bugs.gentoo.org/560396
Reported-by: Jeroen Roovers <jer@gentoo.org>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Make sure we properly check the target of symlinks even when the target
does not exist. This caused problems in two ways:
(1) It allowed code to bypass checks by writing through a symlink that
was in a good location but pointed to a bad (non-existent) location.
(2) It caused code to be wrongly rejected when it tried writing to a
symlink in a bad location but pointed to a good location.
In order to get this behavior, we need to use the new gnulib helpers
added in the previous commit. They include functions which can look
up the targets of symlinks even when the final path doesn't exist.
URL: https://bugs.gentoo.org/540828
Reported-by: Rick Farina <zerochaos@gentoo.org>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
| |
This way we can quickly execute the tests that run dynamic or static
binaries. We leave scripts out as they're a bit of a special case.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
If the stty step fails (for any reason really), the main testrunner
will abort with a weird error message:
$ make check
...
/bin/sh './testsuite' AUTOTEST_PATH='src:tests' --jobs=`getconf _NPROCESSORS_ONLN || echo 1`
testsuite: error: invalid content: atlocal
...
Make sure we ignore stty's exit status, and we put a final comment/$?
reset at the end of the script.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
By allowing the SANDBOX_MESSAGE_PATH var to be stored in the shell
environment and then modified on the fly, we run into a fun edge
case with the PM. When a phase has finished running, it saves the
current environment. When the next phase runs, it loads the env
from the previous run. Since the message path var can contain a
pid, the previous run will no longer be valid.
Since we want this to simply be a way for the active sandbox to
pass information to the active libsandbox.so's, there's no need
to use an env var that the shell can save/reload. As such, use a
variable name that the shell will skip. Non-shell programs have
no problem with this.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
While we took pains to preserve the LD_PRELOAD setting, this doesn't
help us too much in practice. If a process is going out of its way
to blow away LD_PRELOAD, chances are good it's blowing away all vars
it doesn't know about. That means all of our SANDBOX_XXX settings.
Since a preloaded libsandbox.so is useless w/out its SANDBOX_XXX
env vars, make sure we preserve those as well.
These changes also imply some behavioral differences from older
versions. Previously, you could `unset` a sandbox var in order
to disable it. That no longer works. If you wish to disable
things, you have to explicitly set it to "".
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Currently, if a non-static app sets up a pipe (with cloexec enabled) and
executes a static app, the handle to that pipe is left open in the parent
process. This causes trouble when the parent is waiting for that to be
closed immediately.
Since none of the fds in the forked parent process matter to us, we can
just go ahead and clean up all fds before we start tracing the child.
URL: http://bugs.gentoo.org/364877
Reported-by: Victor Stinner <victor.stinner@haypocalc.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This is used whenever sandbox wants to display an informational message.
For example, early notification of a path violation, or debugging output.
We can't just pop open an fd and pass that around as apps consider that
leakage and will often break assumptions in terms of free fds. Or apps
that start up and cleanse all of their open fds.
So instead, we just pass around an env var that holds the full path to
the file we want will write to. Since these messages are infrequent
(compared to overall runtime), opening/writing/closing the path every
time is fine.
This also avoids all the problems associated with using external portage
helpers for writing messages.
A follow up commit will take care of the situation where apps (such as
scons) attempt to also cleanse the env before forking.
URL: http://bugs.gentoo.org/278761
URL: http://bugs.gentoo.org/431638
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
|
|
|
| |
When it comes to processing errors, an empty path is checked before
an invalid dirfd. Make sure sandbox matches that behavior for the
random testsuites out there that look for this.
URL: https://bugs.gentoo.org/346929
Reported-by: Marien Zwart <marienz@gentoo.org>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
|
|
| |
We don't check for O_NOFOLLOW in the open wrappers, so we end up
returning the wrong error when operating on broken symlinks.
URL: https://bugs.gentoo.org/413441
Reported-by: Marien Zwart <marienz@gentoo.org>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
| |
Makes it easier to quickly figure out how to run a helper test
without having to resort to existing usage or the code itself.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
| |
If this flag is set, then the tests get all hung up. Clear
it in case someone has it active on their terminal.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
|
|
|
|
| |
We can trace x32 when the host is x86_64 or x32, but x32 cannot trace
x86_64 due to limitations in the kernel interface -- all pointers get
truncated to 32bits. We'll have to add external ptrace helpers in the
future to make this work, but for now, we'll just let x86_64 code run
unchecked :(.
URL: https://bugs.gentoo.org/394179
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
| |
Since all system headers are included by way of headers.h, we can
pre-compile this to speed up the build up a bit.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
|
|
|
| |
This adds support for signed ll, unsigned z, l, and ll, hex l, and ll,
ignores the # for hex output since this is what we do implicitly already.
As for testing, looks like during the autogeneration of testsuite.list.at,
the sb_printf test was lost. Restore it so it gets run again.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
| |
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
There are a few major points we want to hit here:
- have all output from libsandbox go through portage helpers when we are
in the portage environment so that output is properly logged
- convert SB_E{info,warn,error} to sb_e{info,warn,error} to match style
of other functions and cut down on confusion
- move all abort/output helpers to libsbutil so it can be used in all
source trees and not just by libsandbox
- migrate all abort points to the centralized sb_ebort helper
Unfortunately, it's not terribly easy to untangle these into separate
patches, but hopefully this shouldn't be too messy as much of it is
mechanical: move funcs between files, and change the name of funcs
that get called.
URL: http://bugs.gentoo.org/278761
Reported-by: Mounir Lamouri <volkmar@gentoo.org>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
|
|
|
|
|
| |
To simplify testing, do not mung exit(0) to exit(1) just because the log
file exists. In many of our tests, we will be doing things to generate
a log file, but we explicitly test for exit values ourselves.
This is also needed to make log file handling more resilient where we
get the name at startup, but don't allow live env changes after that.
The changing of the log name to sb.log on the fly no longer works.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
|
| |
Since none of our tests care about the verbose output, move the disable
to a common location so we don't have to do it on a more fine grained
basis.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
|
| |
Almost no one has beep support turned on anymore, and ebeep in the main
tree has been deprecated (meaning it wasn't found useful while building
packages). So punt support for it from sandbox too.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
|
|
| |
Make sure that when we trace static apps, their bad syscalls don't
get a chance to actually complete.
URL: http://bugs.gentoo.org/406543
Reported-by: Marijn Schouten <hkbst@gentoo.org>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
| |
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
| |
Without this, the test will fail because it can't find sandbox.sh
which lives in the src dir.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
| |
POSIX doesn't specify EBADFD, and EBADF should cover us, and we
don't really need it, so disable for now.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
| |
URL: http://bugs.gentoo.org/374059
Reported-by: Nick Bowler <nbowler@draconx.ca>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
| |
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
| |
The open test got this fix a while ago, but open64 was missed.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
| |
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
| |
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
|
|
| |
Let the default mode for files be 0777 rather than 0 so that the default
creation of files actually works.
Also make the flags part of a dirfd filename actually optional.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
| |
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
| |
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
|
|
|
|
| |
The previous commit (libsandbox: handle dirfd in mkdir/open/unlink *at
prechecks) unified some path checks while unifying the dirfd code, but
prevented valid NULL paths from also being handled. Make sure we still
handle that behavior, and add a test for it to prevent future regressions.
URL: http://bugs.gentoo.org/346815
Reported-by: Jake Todd <jaketodd422@gmail.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
| |
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
| |
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|
|
|
|
|
|
|
|
| |
If sigsuspend-zsh_tst fails, it isn't quite clear as to why. So make the
output a bit more clear as to what's going on.
URL: http://bugs.gentoo.org/339022
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
|