Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 96782 - sys-apps/sandbox is full of tempfile races
Summary: sys-apps/sandbox is full of tempfile races
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Security
Classification: Unclassified
Component: Vulnerabilities (show other bugs)
Hardware: All Other
: High normal (vote)
Assignee: Gentoo Security
URL:
Whiteboard: A3 [glsa]
Keywords:
Depends on:
Blocks:
 
Reported: 2005-06-22 06:43 UTC by Tavis Ormandy (RETIRED)
Modified: 2005-08-15 22:02 UTC (History)
3 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tavis Ormandy (RETIRED) gentoo-dev 2005-06-22 06:43:31 UTC
the sandbox utility is full of toctou races, these are unacceptable and should be fixed.

the toctous are of the nature:

is_file_secure(foo);
continue_being_insecure();

(note: i'll work on a patch soon).
Comment 1 Chris C. Blockschmidt 2005-06-23 15:14:19 UTC
While you're at that, please review Bug #47167. That bug being invalid was
unacceptable last year but the situation has unimproved. Nowadays success rate 
increased from 5/8 to nearly 100%.
Comment 2 Tavis Ormandy (RETIRED) gentoo-dev 2005-06-23 15:34:49 UTC
Ughh
Comment 3 SpanKY gentoo-dev 2005-06-23 15:58:00 UTC
do you care to provide any real information ?  simply saying 'hey, sandbox
sucks' isnt useful
Comment 4 Tavis Ormandy (RETIRED) gentoo-dev 2005-06-23 16:42:51 UTC
For example:

main():
...
if (file_exist(sandbox_pids_file, 1) < 0) {
...
} else {
 pids_file = file_open(sandbox_pids_file, "r+", 1, 0664, "portage");
...
}
...

file_open():
...
/* file_security_check() runs some tests then returns wether it's safe to 
proceed, of course, there is no guarantee the filesystem hasnt changed since the 
stat() was performed by f_s_c(), the results may not still be true */
file_security_check(filename);
...
 fd = open(filename, file_getmode(mode));
 file_security_check(filename);
...

where file_getmode(mode) resolves to O_RDWR | O_CREAT in this case, which is a 
TOCTOU race, vulnerable to linking attacks. A solution in this case could be to 
check if the file exists, if it doesnt, use O_EXCL, which would fail if anyone 
tried to win the race.
Comment 5 Chris C. Blockschmidt 2005-06-24 18:12:05 UTC
Why do you need /tmp/sandboxpids.tmp, anyway? IIRC, I think it was the reporter
of bug 21923 (though I'll have to check) who first discovered in may 2003 that
sandboxpids.tmp is only 'needed' when sandbox is injected via 
/etc/ld.so.preload using sandboxpid's.tmp as some sort of reference count... 
Sandbox adds itself to that file at startup (but only if it's the first 
instance) and removes itself when it's finished (but only if it's the last 
instance). Nowadays (even two years ago) sandbox does not use /etc/ld.so.
preload, 
so sandboxpids.tmp does not serve any useful purpose (checked it myself with 
today's sandbox, all sandbox does is add and delete a single number to this 
file but does not use it in any way for anything useful). Logfiles, I think 
nobody I talked to could think of any reason why having those in /tmp would be
a good idea.
Comment 6 Martin Schlemmer (RETIRED) gentoo-dev 2005-06-24 18:59:59 UTC
Ill check the pids file - I am slowly cleaning out the dead code (as you noted,
all the ld.so.preload stuff is out), but I have not really given that much
attention, so cannot say for sure.

As for the logs ... Might move them to /var/log/sandbox/, but just having them
in /var/log/ will bloat things a bit.  I am not a FHS expert, so somebody else
might want to comment.
Comment 7 SpanKY gentoo-dev 2005-06-24 22:27:44 UTC
FHS doesnt say much about /var/log except that it's for logfiles ;)

/var/log/sandbox/ sounds good to me
Comment 8 Martin Schlemmer (RETIRED) gentoo-dev 2005-07-08 16:47:19 UTC
Ok, please check the latest code, and let me know if there are issues.

 $ svn co svn+ssh://svn.gentoo.org/var/svnroot/path-sandbox
Comment 9 Chris C. Blockschmidt 2005-07-08 18:04:10 UTC
There's something else fishy going on since sandbox-1.2.9 - paths longer than
PATH_MAX don't work at all and sandbox is caught in an endless loop (100% cpu).
After studying Bug #21766 (comment  and 7) I think portage's getcwd is broken
again. CC'ing (with permission) writer of Bug #21766 Comment #7 (you are the 
one who came up with the idea for portage's own getcwd, seems like the older
one can handle unlimited paths and the newer one does not. opinion valued)
Comment 10 bartron 2005-07-08 18:24:39 UTC

    
Comment 11 bartron 2005-07-08 18:24:39 UTC
  Well, it can't handle "unlimited" length paths... you still need to 
`stat()' every path component either relative to `/' or `.'... nice 
thing about the glibc version is that it goes backwards from `.' and 
needs only two characters (`..') to go back one level as opposed to 
the full directory name in the uclibc version.  In the particular 
scenario used in Bug 21766 comment #4, ratio is 9/3, which means 
glibc `getcwd()' can handle close to (PATH_MAX * 3) while uclib's 
version is approx. (PATH_MAX * 1)...  no real gain here.


  After closer inspection, other reasons why the version in sandbox-1.2.9+
can't work are...

(1) in `__syscall_egetcwd()', members of `struct stat st' are passed 
uninitialized to `recurser()'... `st' should be initialized to "/".

(2) line #131 overwrites `stat_buf' with "/.." ... instead it should be 
appending it (`strcat()' like in uclibs is perfectly save... previous 
statement already makes sure there's enough room left).

(3) this particular implementation requires that the passed in buffer 
points to an existing directory... use "." for best results...
Comment 12 bartron 2005-07-08 18:27:32 UTC

    
Comment 13 bartron 2005-07-08 18:27:32 UTC
  However, since this is not related to tempfile races, see bug #98419
Comment 14 Martin Schlemmer (RETIRED) gentoo-dev 2005-07-09 02:08:42 UTC
Well, even the glibc one fails with the specific test (handles a bit longer
paths, but not the depth that coreutils test requires), is rather ugly and at
some point redo the whole path with 'confdir3':

  http://bugs.gentoo.org/show_bug.cgi?id=21766#c39

As for the recursion .. cannot say I can get it here.  Will look at the new bug
though.
Comment 15 bartron 2005-07-10 18:06:54 UTC

    
Comment 16 bartron 2005-07-10 18:06:54 UTC
  See Bug #98419 for more info on `getcwd'.
Comment 17 Martin Schlemmer (RETIRED) gentoo-dev 2005-07-11 00:11:52 UTC
Right, besides getcwd() .. can somebody from security comment on the original
problem?
Comment 18 Tavis Ormandy (RETIRED) gentoo-dev 2005-07-13 07:55:26 UTC
Looks good, the logs are all moved into /var/log?

seems home_dir can be set to /tmp if HOME has been unset? is that ever likely to 
happen? and if so, does it read or write anything in there? (sorry, unfamiliar 
with it's operation :)

If that's not an issue, everything looks fine to me.
Comment 19 Martin Schlemmer (RETIRED) gentoo-dev 2005-07-13 08:32:44 UTC
Can do.  It should not anymore .. its more to make sure we have a sane TMPDIR
and its writable for ./configure scripts, etc ...
Comment 20 Martin Schlemmer (RETIRED) gentoo-dev 2005-07-15 03:01:10 UTC
Actually, it already sets, home_dir to /tmp if all else fails.
Comment 21 Thierry Carrez (RETIRED) gentoo-dev 2005-07-18 05:16:20 UTC
Martin: do you have any idea of the release date for the fix ?
Comment 22 Martin Schlemmer (RETIRED) gentoo-dev 2005-07-20 06:02:46 UTC
Thierry, that should be already in 1.2.11.
Comment 23 Thierry Carrez (RETIRED) gentoo-dev 2005-07-20 06:27:43 UTC
Ah ok. Great :)

Any reason why we should not ask arch teams to test and stable-ize ? Or do you
prefer to wait a little in ~ to catch bugs ?
Comment 24 Thierry Carrez (RETIRED) gentoo-dev 2005-07-20 06:46:00 UTC
Arches, please test sandbox-1.2.11 carefully and mark stable if it is.
Comment 25 Danny van Dyk (RETIRED) gentoo-dev 2005-07-20 09:26:23 UTC
amd64 stable
Comment 26 Gustavo Zacarias (RETIRED) gentoo-dev 2005-07-20 10:43:10 UTC
sparc stable.
Comment 27 Markus Rothe (RETIRED) gentoo-dev 2005-07-20 13:32:27 UTC
stable on ppc64
Comment 28 Joe Jezak (RETIRED) gentoo-dev 2005-07-20 19:28:18 UTC
Tested and marked ppc stable.
Comment 29 Chris Gianelloni (RETIRED) gentoo-dev 2005-07-21 08:15:48 UTC
Tested and marked x86 stable
Comment 30 René Nussbaumer (RETIRED) gentoo-dev 2005-07-21 11:52:43 UTC
Stable on hppa. Sorry for the delay.
Comment 31 Chris Gianelloni (RETIRED) gentoo-dev 2005-07-21 12:14:16 UTC
Alpha and IA64 were done by agriffis
Comment 32 Sune Kloppenborg Jeppesen gentoo-dev 2005-07-23 04:47:33 UTC
This one is ready for GLSA. 
Comment 33 Sune Kloppenborg Jeppesen gentoo-dev 2005-07-25 11:32:57 UTC
GLSA 200507-22 
 
arm and s390 don't forget to mark stable.