Bugzilla Bug 21923

If sandbox's pid and logfiles already exist when sandbox is about to create
them, it only checks for them to be regular files (to prevent symlink attacks).
However, if "/" and "/tmp" are on the same filesystem (really bad idea, but
seems very common), hard links may be used to get the same result.


----------------
#as root, create a test file

root# echo hello > /etc/hello
root#

--------
#as normal user:

$ ls -adl /etc
drwxr-xr-x          29 root root     1024 May 29 23:25 /etc/
$ ls -ail /etc/hello
219781 -rw-r--r--    2 root root        6 May 29 23:25 /etc/hello
$ cd /tmp
$ ls -adl /tmp
drwxrwxrwt           9 root root     1024 May 29 23:25 /tmp/
$ ln /etc/hello sandboxpids.tmp
$ ls -ail sandboxpids.tmp
219781 -rw-r--r--    2 root root    6 May 29 23:26 sandboxpids.tmp
$ cat sandboxpids.tmp
hello
$
#now wait for root to use emerge

--------
#as root:

root# emerge info | grep FEATURES
FEATURES="sandbox"
root# emerge some_pkg
[...]
root# ls -ail /etc/hello
219781 -rw-r--r--    1 root root    0 May 29 23:27 /etc/hello
root#
----------------

files created by sandbox:

* "/tmp/sandboxpids.tmp": created on every invokation and deleted afterwards,
always created with the same name; link destination will be emptied (see
above).

* "/tmp/sandbox-<pid>.log": filename not known until sandbox's pid is known,
but sandbox does not create it until output occurs. The link's destination will
have sandbox's output appended to it.

* "/tmp/sandbox-debug-.log": hmm.. I'm a bit unclear about this one... the
source has the file name composed as:
  "%s%s%s", DEBUG_LOG_FILE_PREFIX, pid_string, LOG_FILE_EXT,
however, `pid_string' will always be "\0" at this point (or rather,
uninitialized, but since it's on main()'s stack, uninitialized means zeroed).
Output only happens if `SANDBOX_DEBUG' is set, and is appended to the link's
destination.

========

Second problem, sandbox doesn't detect regular files created by somebody else
(works with sandbox-*.log too)

$ cd /tmp
$ touch sandboxpids.tmp
$ chmod 666 sandboxpids.tmp
$ ls -al sandboxpids.tmp
-rw-rw-rw-    1 bartron  users  0 May 29 23:27 sandboxpids.tmp

root# /usr/lib/portage/bin/sandbox
root# cd /tmp
root# ls -al sandboxpids.tmp
-rw-rw-rw-    1 bartron  users  6 May 29 23:27 sandboxpids.tmp

A malicious user may prefill the logfile with random garbage, or modify/delete
it while sandbox is running (it's not open all the time!). (print_sandbox_log()
will allocates a block of memory big enough to hold the entire logfile, then
prints it to stdout). This even works when sandbox is not running as root. 

--------

Suggested changes:

* Check directory permissions first. If it's not owned by root.root OR is
writable by everyone and doesn't have the sticky bit set, DON'T use that
directory!
* If the file in question already exists, make sure it's a regular file
(already done), its link count is 1, it's owned by root (or portage, or whoever
sandbox is running as) and writable by that uid only. If it does not exist,
open it nondestructively, and perform the same checks as above (plus, make sure
fstat() and lstat() results are the same).

However, the way these files are used (open only while they're accessed;
constant or easily guessed names), "/tmp" doesn't seem like a good place to put
them in the first place (if i'm not missing something (haven't looked at all
parts of portage yet), sandbox runs as either root.root or portage.portage, so
a private directory, like "/var/*/portage, rwxrwx--- root.portage" would seem
like a better location.)

:bartron/Thomas


Reproducible: Always
Steps to Reproduce:

For the first issue ... it removes the sandboxpids.tmp if its the only running
instance, so that solves the issue.  Same for the log files (excepts it always
remove them at instance start - might just want to verify that).

It is however an issue (with sandboxpids.tmp) if it is not the first (only)
instance of sandbox running.  I will have a look and try to get a more
secure fix going.

I can confirm this bug. Tried it this morning on a box without a separate /tmp 
partition and I could empty out any file on the same partition (except those 
belonging to running processes):

--8<--

$ cd /tmp
$ ln /sbin/ldconfig sandboxpids.tmp
$ ls -l /sbin/ldconfig
-rwxr-xr-x    2 root     root       450340 Jul 20 03:21 /sbin/ldconfig
$ ls -l /tmp/sandboxpids.tmp 
-rwxr-xr-x    2 root     root       450340 Jul 20 03:21 /tmp/sandboxpids.tmp

# emerge ufed
<emerge output snipped>

$ ls -l /sbin/ldconfig
-rwxr-xr-x    1 root     root            0 Aug  1 15:05 /sbin/ldconfig
$ ls -l /tmp/sandboxpids.tmp 
ls: /tmp/sandboxpids.tmp: No such file or directory

--8<--

As there are no traces as to what might have happened, I think this is 
rather dangerous!

Using portage-2.0.48-r5.

any news on this ?

*** Bug 41708 has been marked as a duplicate of this bug. ***

http://gentoo.twobit.net/misc/sandbox-security-040404.diff

Will be applied once it's known to be proper. Feel free to
test it and report back.

http://gentoo.twobit.net/misc/sandbox-security-040404-3.diff

In the file_open() call it runs a security check beyond the quick regular check that exists in file_exists():

lstat the file, check that it actually exists or return.
Check that the number of hardlinks is not greater than 1, else unlink.
Check that the file is not a symlink, else exit with error.
Check that the file is a regular file, else exit with error.
Check that the file is not world-writable, else unlink.
Check that the UID and GID are in [root, getuid(), portage], else unlink.

If it can't unlink... It bails with an error.

That should cover everything, no?

http://gentoo.twobit.net/misc/sandbox-security-040404-4.diff

SpanKY pointed out a potential for a race condition between
the unlink() and the fopen() calls. Added a second pass that
ensures no foul play has occured. It bails on failure.

This is available as portage-2.0.50-r3 which is the current stable portage.

Okay -- is it safe to reassign this to security@ for a GLSA?

Reassigning...

GLSA sent out; http://article.gmane.org/gmane.linux.gentoo.announce/306.
Closing bug as FIXED.