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.