Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 21923 - [portage/sandbox-1.1] - security issues with file creation in /tmp
Summary: [portage/sandbox-1.1] - security issues with file creation in /tmp
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Security
Classification: Unclassified
Component: GLSA Errors (show other bugs)
Hardware: All Linux
: High critical (vote)
Assignee: Gentoo Security
URL:
Whiteboard:
Keywords: InVCS
: 41708 (view as bug list)
Depends on:
Blocks:
 
Reported: 2003-05-29 18:39 UTC by bartron
Modified: 2004-04-06 11:34 UTC (History)
4 users (show)

See Also:
Package list:
Runtime testing required: ---
plasmaroo: Pending-
plasmaroo: Assigned_To? (plasmaroo)


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description bartron 2003-05-29 18:39:49 UTC
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:
Comment 1 Martin Schlemmer (RETIRED) gentoo-dev 2003-06-30 15:15:58 UTC
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.
Comment 2 Reporter 2003-08-01 08:39:00 UTC
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!
Comment 3 Reporter 2003-08-01 08:45:20 UTC
Using portage-2.0.48-r5.
Comment 4 Marius Mauch (RETIRED) gentoo-dev 2004-01-11 21:07:56 UTC
any news on this ?
Comment 5 Jon Portnoy (RETIRED) gentoo-dev 2004-04-03 19:48:16 UTC
*** Bug 41708 has been marked as a duplicate of this bug. ***
Comment 6 Nicholas Jones (RETIRED) gentoo-dev 2004-04-04 11:04:09 UTC
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.
Comment 7 Nicholas Jones (RETIRED) gentoo-dev 2004-04-04 12:51:25 UTC
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?
Comment 8 Nicholas Jones (RETIRED) gentoo-dev 2004-04-04 13:26:53 UTC
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.
Comment 9 Nicholas Jones (RETIRED) gentoo-dev 2004-04-04 23:35:48 UTC
This is available as portage-2.0.50-r3 which is the current stable portage.
Comment 10 Jon Portnoy (RETIRED) gentoo-dev 2004-04-05 17:40:03 UTC
Okay -- is it safe to reassign this to security@ for a GLSA?
Comment 11 Tim Yamin (RETIRED) gentoo-dev 2004-04-06 02:30:05 UTC
Reassigning...
Comment 12 Tim Yamin (RETIRED) gentoo-dev 2004-04-06 11:34:14 UTC
GLSA sent out; http://article.gmane.org/gmane.linux.gentoo.announce/306. Closing bug as FIXED.