uses unsafe /tmp files

Bug #445105 reported by Kees Cook on 2009-10-07
260
This bug affects 1 person
Affects Status Importance Assigned to Milestone
eucalyptus (Ubuntu)
High
Dustin Kirkland 
Karmic
High
Dustin Kirkland 

Bug Description

Split from bug 436977; in some places, eucalyptus uses unsafe /tmp files:

./gatherlog/GLclient.c: env = axutil_env_create_all("/tmp/fooh", AXIS2_LOG_LEVEL_TRACE);
./install-sh: tmpdir=${TMPDIR-/tmp}/ins$RANDOM-$$
./storage/storage.c:#define F1 "/tmp/improbable-cache-file-1"
./storage/storage.c:#define F2 "/tmp/improbable-cache-file-2"
./storage/storage.c:#define F3 "/tmp/improbable-cache-file-3"
./storage/storage.c:#define F4 "/tmp/improbable-cache-file-4"
./storage/storage.c:#define F5 "/tmp/improbable-cache-file-5"
./storage/storage.c:#define RM_CMD "rm -rf /tmp/improbable-cache-file-?"
./cluster/CCclient.c: env = axutil_env_create_all("/tmp/fofo", AXIS2_LOG_LEVEL_TRACE);
./tools/detach.pl:system("cp $virshxmlfile /tmp/wtf");
./tools/httpd.conf:ServerRoot "/tmp"
./tools/euca_watchdog.pl:our $chkpt_file = "/tmp/euca_watchdog.checkpoint";
./node/handlers_xen.c: snprintf(filename, 1024, "/tmp/consoleOutput.%s", instanceId);
./debian/patches/var_lib_eucalyptus.diff: HTTPD_HOME="/tmp/"

ProblemType: Bug
Architecture: amd64
Date: Tue Oct 6 20:09:05 2009
DistroRelease: Ubuntu 9.10
Package: eucalyptus-common (not installed)
ProcEnviron:
 LANGUAGE=en_US.UTF-8
 PATH=(custom, user)
 LANG=en_US.UTF-8
 SHELL=/bin/bash
ProcVersionSignature: Ubuntu 2.6.31-11.36-generic
SourcePackage: eucalyptus
Uname: Linux 2.6.31-11-generic x86_64

Matt Zimmerman (mdz) on 2009-10-08
Changed in eucalyptus (Ubuntu):
importance: Undecided → High
status: New → Triaged
Kees Cook (kees) on 2009-10-13
visibility: private → public
Changed in eucalyptus (Ubuntu Karmic):
milestone: none → ubuntu-9.10
Thierry Carrez (ttx) on 2009-10-14
tags: added: eucalyptus
Thierry Carrez (ttx) on 2009-10-14
Changed in eucalyptus (Ubuntu Karmic):
assignee: nobody → Thierry Carrez (ttx)
Daniel Nurmi (nurmi) wrote :

Greetings,

We've analyzed the points in the source tree that are referenced, and have found that none of them are actually exercised in a Karmic default eucalyptus running system. More detail on each follows:

GLclient is a testing utility that doesn't get installed by the package (or a 'make install' for that matter)
./gatherlog/GLclient.c: env = axutil_env_create_all("/tmp/fooh", AXIS2_LOG_LEVEL_TRACE);

install-sh is only used for building eucalyptus
./install-sh: tmpdir=${TMPDIR-/tmp}/ins$RANDOM-$$

the function in 'storage.c' that uses these files is only used by 'test.c' in 'storage/', which is a utility that is never installed
./storage/storage.c:#define F1 "/tmp/improbable-cache-file-1"
./storage/storage.c:#define F2 "/tmp/improbable-cache-file-2"
./storage/storage.c:#define F3 "/tmp/improbable-cache-file-3"
./storage/storage.c:#define F4 "/tmp/improbable-cache-file-4"
./storage/storage.c:#define F5 "/tmp/improbable-cache-file-5"
./storage/storage.c:#define RM_CMD "rm -rf /tmp/improbable-cache-file-?"

CCclient is a debugging utility that is not installed
./cluster/CCclient.c: env = axutil_env_create_all("/tmp/fofo", AXIS2_LOG_LEVEL_TRACE);

These two are never used for Karmic by default (which uses the handlers_kvm.c), unless a user installs Xen on their node controllers. The first can be entirely removed (is just there for debugging). The second is actually, I believe, as unpredictable or more so than a file created with mktemp(). Eucalyptus instanceIds are random unique ids of 8 hex characters ("i-ABCDEFGH" where A-H are hex values).
./tools/detach.pl:system("cp $virshxmlfile /tmp/wtf");
./node/handlers_xen.c: snprintf(filename, 1024, "/tmp/consoleOutput.%s", instanceId);

The httpd*.conf that ends up actually being used has this value set to '/' instead of '/tmp'. The init script(s) actually replace this with '/'.
./debian/patches/var_lib_eucalyptus.diff: HTTPD_HOME="/tmp/"
./tools/httpd.conf:ServerRoot "/tmp"

This tool is never installed.
./tools/euca_watchdog.pl:our $chkpt_file = "/tmp/euca_watchdog.checkpoint";

That's great news, Dan.

Could you commit a single revision that cleans these up? I'll merge
into our tree and upload immediately...

:-Dustin

Kees Cook (kees) wrote :

On Thu, Oct 15, 2009 at 06:01:44PM -0000, Daniel Nurmi wrote:
> We've analyzed the points in the source tree that are referenced, and
> have found that none of them are actually exercised in a Karmic default
> eucalyptus running system. More detail on each follows:

Excellent, the bulk of these can be ignored then. :)

> These two are never used for Karmic by default (which uses the handlers_kvm.c), unless a user installs Xen on their node controllers. The first can be entirely removed (is just there for debugging). The second is actually, I believe, as unpredictable or more so than a file created with mktemp(). Eucalyptus instanceIds are random unique ids of 8 hex characters ("i-ABCDEFGH" where A-H are hex values).
> ./tools/detach.pl:system("cp $virshxmlfile /tmp/wtf");

Yeah, this line should just be removed.

> ./node/handlers_xen.c: snprintf(filename, 1024, "/tmp/consoleOutput.%s", instanceId);

Would it make sense to use some other location instead of /tmp? While
it's not predictable, the file isn't handled safely on creation. At the
least, I would recommend adding O_EXCL to the flags. This may require
an unlink() call to blow away old files, but that's symlink-attack safe.
With the O_EXCL, worst case is a DoS on the console output.

Speaking of that code, I noticed this on the side that waits for the file
to appear:
    while(count < 10000 && stat(filename, &statbuf) < 0) {count++;}
    fd = open(filename, O_RDONLY);

That loop is going to go by _very_ quickly. e.g. on my system, that takes
0.017 seconds to execute. I would recommend (not for security, but for
possible race bugs) that you use nanosleep() and time() for choosing an
actual timeout to wait with. :)

Thanks for checking!

-Kees

--
Kees Cook
Ubuntu Security Team

Kees Cook (kees) on 2009-10-15
Changed in eucalyptus (Ubuntu Karmic):
assignee: Thierry Carrez (ttx) → Kees Cook (kees)
status: Triaged → In Progress
Kees Cook (kees) wrote :
Changed in eucalyptus (Ubuntu Karmic):
status: In Progress → Fix Committed
Changed in eucalyptus (Ubuntu Karmic):
assignee: Kees Cook (kees) → Dustin Kirkland (kirkland)
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package eucalyptus - 1.6~bzr931-0ubuntu4

---------------
eucalyptus (1.6~bzr931-0ubuntu4) karmic; urgency=low

  [ Kees Cook ]
  * Fix unsafe /tmp file uses (LP: #445105):
    - tools/detach.pl: remove debugging XML dump call.
    - node/handlers_xen.c: use mkstemp().

  [ Mathias Gug ]
  * debian/eucalyptus-common.eucalyptus.upstart: Start eucalyptus-cloud
    with all the relevant options (LP: #452665).

  [ Dustin Kirkland ]
  * cluster/handlers.c, net/vnetwork.c: label the 169.254.169.254 link
    local address with :metadata, the public vm ip addresses with :pub,
    and the private vm ip addresses with :priv, to separate it appropriately
    (LP: #452754)
  * debian/eucalyptus-ipaddr.conf: and now that we're separating the weird
    addresses to their own labels, we can show just the device label, much
    cleaner output, (LP: #451607)
  * debian/copyright: Eucalyptus licensed 1.6 under the GPLv3, update
    our copyright file accordingly, (LP: #453129)
  * debian/control, debian/eucalyptus-java-common.links: the CLC absolutely
    needs the ecj.jar installed and linked for euca-* and ec2-* API calls
    to work (LP: #453177)

 -- Dustin Kirkland <email address hidden> Fri, 16 Oct 2009 13:28:32 -0500

Changed in eucalyptus (Ubuntu Karmic):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public Security information  Edit
Everyone can see this security related information.

Other bug subscribers