summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorVMware, Inc <>2013-09-17 20:39:34 -0700
committerDmitry Torokhov <dmitry.torokhov@gmail.com>2013-09-22 22:26:43 -0700
commit3a9f2297a82b9c109e894b5f8ea17753e68830ac (patch)
treee1544d769543ade25ff5157d1a2687952f48178d
parent3869012deb7658b9aab10ab028d71b32b89a2a85 (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.c23
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;