diff options
author | VMware, Inc <> | 2013-09-17 20:39:34 -0700 |
---|---|---|
committer | Dmitry Torokhov <dmitry.torokhov@gmail.com> | 2013-09-22 22:26:43 -0700 |
commit | 3a9f2297a82b9c109e894b5f8ea17753e68830ac (patch) | |
tree | e1544d769543ade25ff5157d1a2687952f48178d | |
parent | 3869012deb7658b9aab10ab028d71b32b89a2a85 (diff) |
Harden HostinfoOSData against $PATH attacks.
We are doing a popen("lsb_release... ") when attempting to
determine host details in hostinfoPosix.c. Using popen means that
$PATH is walked when looking for the lsb_release binary, and that
may give an attacker the ability to run a malicious version of
lsb_release.
This change does two things,
a) Hard code the path to lsb_release. I've searched around
the web and I believe the path is always "/usr/bin/lsb_release"
so let's not leave this up to chance.
b) Stop running HostinfoGetCmdOutput with elevated privileges. Drop
to non-root when possible. If someone sneaks in a new call to
HostinfoGetCmdOutput and doesn't use a full path, then we will
hopefully avoid a firedrill. I'm only applying this to Linux
because the Fusion build barfed when I tried to compile with
without the vmx86_linux.
I think either (a) or (b) would be enough but I'm doing both,
because each individually is correct. Also note that in the blog
post by Tavis Ormandy calls out doing (a) as not enough,
http://blog.cmpxchg8b.com/2013/08/security-debianisms.html
His example uses a bash feature that allows functions to be
exported. I haven't been able to get that to work on my Ubuntu
machine.
To test I'm manually run Linux WS and Fusion and verified that
the logs look correct.
Signed-off-by: Dmitry Torokhov <dtor@vmware.com>
-rw-r--r-- | open-vm-tools/lib/misc/hostinfoPosix.c | 23 |
1 files changed, 19 insertions, 4 deletions
diff --git a/open-vm-tools/lib/misc/hostinfoPosix.c b/open-vm-tools/lib/misc/hostinfoPosix.c index af492683..0fe00339 100644 --- a/open-vm-tools/lib/misc/hostinfoPosix.c +++ b/open-vm-tools/lib/misc/hostinfoPosix.c @@ -763,17 +763,27 @@ out: static char * HostinfoGetCmdOutput(const char *cmd) // IN: { + Bool isSuperUser = FALSE; DynBuf db; FILE *stream; char *out = NULL; + /* + * Attempt to lower privs, because we use popen and an attacker + * may control $PATH. + */ + if (vmx86_linux && Id_IsSuperUser()) { + Id_EndSuperUser(getuid()); + isSuperUser = TRUE; + } + DynBuf_Init(&db); stream = Posix_Popen(cmd, "r"); if (stream == NULL) { Warning("Unable to get output of command \"%s\"\n", cmd); - return NULL; + goto exit; } for (;;) { @@ -807,11 +817,16 @@ HostinfoGetCmdOutput(const char *cmd) // IN: if (DynBuf_Get(&db)) { out = (char *) DynBuf_AllocGet(&db); } - closeIt: - DynBuf_Destroy(&db); + closeIt: pclose(stream); + exit: + DynBuf_Destroy(&db); + + if (isSuperUser) { + Id_BeginSuperUser(); + } return out; } @@ -930,7 +945,7 @@ HostinfoOSData(void) * Try to get OS detailed information from the lsb_release command. */ - lsbOutput = HostinfoGetCmdOutput("lsb_release -sd 2>/dev/null"); + lsbOutput = HostinfoGetCmdOutput("/usr/bin/lsb_release -sd 2>/dev/null"); if (!lsbOutput) { int i; |