From 8af57380c9ea6f6ffec579804a409e54d323f39a Mon Sep 17 00:00:00 2001

Message-Id: <8af57380c9ea6f6ffec579804a409e54d323f39a.1383321465.git.jdenemar@redhat.com>

From: "Daniel P. Berrange" <berrange@redhat.com>

Date: Wed, 30 Oct 2013 17:01:51 +0000

Subject: [PATCH] Remove all direct use of getenv

For

  https://bugzilla.redhat.com/show_bug.cgi?id=1015247

Unconditional use of getenv is not secure in setuid env.

While not all libvirt code runs in a setuid env (since

much of it only exists inside libvirtd) this is not always

clear to developers. So make all the code paranoid, even

if it only ever runs inside libvirtd.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

(cherry picked from commit 1e4a02bdfe6307f93763fa2c9681f280c564aee5)

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

---

 daemon/libvirtd.c               |  2 +-

 src/driver.c                    |  3 ++-

 src/libvirt.c                   |  2 +-

 src/locking/lock_daemon.c       |  4 ++--

 src/locking/lock_driver_lockd.c |  2 +-

 src/locking/lock_manager.c      |  2 +-

 src/lxc/lxc_driver.c            |  4 ++--

 src/remote/remote_driver.c      |  4 ++--

 src/rpc/virnettlscontext.c      |  4 ++--

 src/util/virauth.c              |  2 +-

 src/util/virfile.c              |  7 +++++--

 src/util/virlog.c               |  8 ++++----

 src/util/virrandom.c            |  2 +-

 src/util/virutil.c              |  8 ++++----

 src/vbox/vbox_XPCOMCGlue.c      |  2 +-

 src/vbox/vbox_tmpl.c            |  4 ++--

 tools/virsh.c                   | 18 +++++++++---------

 17 files changed, 41 insertions(+), 37 deletions(-)

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c

index c9cd1a1..808bd4b 100644

--- a/daemon/libvirtd.c

+++ b/daemon/libvirtd.c

@@ -991,7 +991,7 @@ static int migrateProfile(void)

         goto cleanup;

     }

-    config_home = getenv("XDG_CONFIG_HOME");

+    config_home = virGetEnvBlockSUID("XDG_CONFIG_HOME");

     if (config_home && config_home[0] != '\0') {

         if (VIR_STRDUP(xdg_dir, config_home) < 0)

             goto cleanup;

diff --git a/src/driver.c b/src/driver.c

index a08dd34..ab2a253 100644

--- a/src/driver.c

+++ b/src/driver.c

@@ -27,6 +27,7 @@

 #include "driver.h"

 #include "viralloc.h"

 #include "virlog.h"

+#include "virutil.h"

 #include "configmake.h"

 #include "virstring.h"

@@ -43,7 +44,7 @@ static const char *moddir = NULL;

 void

 virDriverModuleInitialize(const char *defmoddir)

 {

-    const char *custommoddir = getenv("LIBVIRT_DRIVER_DIR");

+    const char *custommoddir = virGetEnvBlockSUID("LIBVIRT_DRIVER_DIR");

     if (custommoddir)

         moddir = custommoddir;

     else if (defmoddir)

diff --git a/src/libvirt.c b/src/libvirt.c

index 4c1992c..0a024a9 100644

--- a/src/libvirt.c

+++ b/src/libvirt.c

@@ -1094,7 +1094,7 @@ virConnectGetDefaultURI(virConfPtr conf,

 {

     int ret = -1;

     virConfValuePtr value = NULL;

-    char *defname = getenv("LIBVIRT_DEFAULT_URI");

+    const char *defname = virGetEnvBlockSUID("LIBVIRT_DEFAULT_URI");

     if (defname && *defname) {

         VIR_DEBUG("Using LIBVIRT_DEFAULT_URI '%s'", defname);

         *name = defname;

diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c

index 77d6e0d..0ac16c3 100644

--- a/src/locking/lock_daemon.c

+++ b/src/locking/lock_daemon.c

@@ -605,7 +605,7 @@ virLockDaemonSetupNetworkingSystemD(virNetServerPtr srv)

     VIR_DEBUG("Setting up networking from systemd");

-    if (!(pidstr = getenv("LISTEN_PID"))) {

+    if (!(pidstr = virGetEnvAllowSUID("LISTEN_PID"))) {

         VIR_DEBUG("No LISTEN_FDS from systemd");

         return 0;

     }

@@ -621,7 +621,7 @@ virLockDaemonSetupNetworkingSystemD(virNetServerPtr srv)

         return 0;

     }

-    if (!(fdstr = getenv("LISTEN_FDS"))) {

+    if (!(fdstr = virGetEnvAllowSUID("LISTEN_FDS"))) {

         VIR_DEBUG("No LISTEN_FDS from systemd");

         return 0;

     }

diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c

index b115799..86ce2d8 100644

--- a/src/locking/lock_driver_lockd.c

+++ b/src/locking/lock_driver_lockd.c

@@ -84,7 +84,7 @@ static virLockManagerLockDaemonDriverPtr driver = NULL;

 static const char *

 virLockManagerLockDaemonFindDaemon(void)

 {

-    const char *customDaemon = getenv("VIRTLOCKD_PATH");

+    const char *customDaemon = virGetEnvBlockSUID("VIRTLOCKD_PATH");

     if (customDaemon)

         return customDaemon;

diff --git a/src/locking/lock_manager.c b/src/locking/lock_manager.c

index aff54c0..f09c168 100644

--- a/src/locking/lock_manager.c

+++ b/src/locking/lock_manager.c

@@ -135,7 +135,7 @@ virLockManagerPluginPtr virLockManagerPluginNew(const char *name,

     void *handle = NULL;

     virLockDriverPtr driver;

     virLockManagerPluginPtr plugin = NULL;

-    const char *moddir = getenv("LIBVIRT_LOCK_MANAGER_PLUGIN_DIR");

+    const char *moddir = virGetEnvBlockSUID("LIBVIRT_LOCK_MANAGER_PLUGIN_DIR");

     char *modfile = NULL;

     char *configFile = NULL;

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c

index 7999b8c..b746ffd 100644

--- a/src/lxc/lxc_driver.c

+++ b/src/lxc/lxc_driver.c

@@ -1362,14 +1362,14 @@ static int lxcStateInitialize(bool privileged,

                               void *opaque ATTRIBUTE_UNUSED)

 {

     virCapsPtr caps = NULL;

-    char *ld;

+    const char *ld;

     virLXCDriverConfigPtr cfg = NULL;

     /* Valgrind gets very annoyed when we clone containers, so

      * disable LXC when under valgrind

      * XXX remove this when valgrind is fixed

      */

-    ld = getenv("LD_PRELOAD");

+    ld = virGetEnvBlockSUID("LD_PRELOAD");

     if (ld && strstr(ld, "vgpreload")) {

         VIR_INFO("Running under valgrind, disabling driver");

         return 0;

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c

index dcd433b..67daf79 100644

--- a/src/remote/remote_driver.c

+++ b/src/remote/remote_driver.c

@@ -187,7 +187,7 @@ remoteFindDaemonPath(void)

         NULL

     };

     size_t i;

-    const char *customDaemon = getenv("LIBVIRTD_PATH");

+    const char *customDaemon = virGetEnvBlockSUID("LIBVIRTD_PATH");

     if (customDaemon)

         return customDaemon;

@@ -955,7 +955,7 @@ remoteConnectOpen(virConnectPtr conn,

 {

     struct private_data *priv;

     int ret, rflags = 0;

-    const char *autostart = getenv("LIBVIRT_AUTOSTART");

+    const char *autostart = virGetEnvBlockSUID("LIBVIRT_AUTOSTART");

     if (inside_daemon && (!conn->uri || (conn->uri && !conn->uri->server)))

         return VIR_DRV_OPEN_DECLINED;

diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c

index 7cee27c..cd69794 100644

--- a/src/rpc/virnettlscontext.c

+++ b/src/rpc/virnettlscontext.c

@@ -710,7 +710,7 @@ static virNetTLSContextPtr virNetTLSContextNew(const char *cacert,

                                                bool isServer)

 {

     virNetTLSContextPtr ctxt;

-    char *gnutlsdebug;

+    const char *gnutlsdebug;

     int err;

     if (virNetTLSContextInitialize() < 0)

@@ -719,7 +719,7 @@ static virNetTLSContextPtr virNetTLSContextNew(const char *cacert,

     if (!(ctxt = virObjectLockableNew(virNetTLSContextClass)))

         return NULL;

-    if ((gnutlsdebug = getenv("LIBVIRT_GNUTLS_DEBUG")) != NULL) {

+    if ((gnutlsdebug = virGetEnvAllowSUID("LIBVIRT_GNUTLS_DEBUG")) != NULL) {

         int val;

         if (virStrToLong_i(gnutlsdebug, NULL, 10, &val) < 0)

             val = 10;

diff --git a/src/util/virauth.c b/src/util/virauth.c

index f58797c..e66cbf5 100644

--- a/src/util/virauth.c

+++ b/src/util/virauth.c

@@ -42,7 +42,7 @@ virAuthGetConfigFilePathURI(virURIPtr uri,

 {

     int ret = -1;

     size_t i;

-    const char *authenv = getenv("LIBVIRT_AUTH_FILE");

+    const char *authenv = virGetEnvBlockSUID("LIBVIRT_AUTH_FILE");

     char *userdir = NULL;

     *path = NULL;

diff --git a/src/util/virfile.c b/src/util/virfile.c

index 7af0843..92a81a5 100644

--- a/src/util/virfile.c

+++ b/src/util/virfile.c

@@ -1354,6 +1354,7 @@ virFileIsLink(const char *linkpath)

 char *

 virFindFileInPath(const char *file)

 {

+    const char *origpath = NULL;

     char *path = NULL;

     char *pathiter;

     char *pathseg;

@@ -1382,9 +1383,11 @@ virFindFileInPath(const char *file)

     }

     /* copy PATH env so we can tweak it */

-    path = getenv("PATH");

+    origpath = virGetEnvBlockSUID("PATH");

+    if (!origpath)

+        origpath = "/bin:/usr/bin";

-    if (VIR_STRDUP_QUIET(path, path) <= 0)

+    if (VIR_STRDUP_QUIET(path, origpath) <= 0)

         return NULL;

     /* for each path segment, append the file to search for and test for

diff --git a/src/util/virlog.c b/src/util/virlog.c

index 694da89..f250679 100644

--- a/src/util/virlog.c

+++ b/src/util/virlog.c

@@ -1637,18 +1637,18 @@ virLogParseDefaultPriority(const char *priority)

 void

 virLogSetFromEnv(void)

 {

-    char *debugEnv;

+    const char *debugEnv;

     if (virLogInitialize() < 0)

         return;

-    debugEnv = getenv("LIBVIRT_DEBUG");

+    debugEnv = virGetEnvAllowSUID("LIBVIRT_DEBUG");

     if (debugEnv && *debugEnv)

         virLogParseDefaultPriority(debugEnv);

-    debugEnv = getenv("LIBVIRT_LOG_FILTERS");

+    debugEnv = virGetEnvAllowSUID("LIBVIRT_LOG_FILTERS");

     if (debugEnv && *debugEnv)

         virLogParseFilters(debugEnv);

-    debugEnv = getenv("LIBVIRT_LOG_OUTPUTS");

+    debugEnv = virGetEnvAllowSUID("LIBVIRT_LOG_OUTPUTS");

     if (debugEnv && *debugEnv)

         virLogParseOutputs(debugEnv);

 }

diff --git a/src/util/virrandom.c b/src/util/virrandom.c

index c233732..6101aac 100644

--- a/src/util/virrandom.c

+++ b/src/util/virrandom.c

@@ -50,7 +50,7 @@ virRandomOnceInit(void)

     /* Normally we want a decent seed.  But if reproducible debugging

      * of a fixed pseudo-random sequence is ever required, uncomment

      * this block to let an environment variable force the seed.  */

-    const char *debug = getenv("VIR_DEBUG_RANDOM_SEED");

+    const char *debug = virGetEnvBlockSUID("VIR_DEBUG_RANDOM_SEED");

     if (debug && virStrToLong_ui(debug, NULL, 0, &seed) < 0)

         return -1;

diff --git a/src/util/virutil.c b/src/util/virutil.c

index a41117e..2b3cbeb 100644

--- a/src/util/virutil.c

+++ b/src/util/virutil.c

@@ -770,7 +770,7 @@ virGetUserDirectoryByUID(uid_t uid)

 static char *virGetXDGDirectory(const char *xdgenvname, const char *xdgdefdir)

 {

-    const char *path = getenv(xdgenvname);

+    const char *path = virGetEnvBlockSUID(xdgenvname);

     char *ret = NULL;

     char *home = NULL;

@@ -798,7 +798,7 @@ char *virGetUserCacheDirectory(void)

 char *virGetUserRuntimeDirectory(void)

 {

-    const char *path = getenv("XDG_RUNTIME_DIR");

+    const char *path = virGetEnvBlockSUID("XDG_RUNTIME_DIR");

     if (!path || !path[0]) {

         return virGetUserCacheDirectory();

@@ -1111,7 +1111,7 @@ virGetUserDirectoryByUID(uid_t uid ATTRIBUTE_UNUSED)

     const char *dir;

     char *ret;

-    dir = getenv("HOME");

+    dir = virGetEnvBlockSUID("HOME");

     /* Only believe HOME if it is an absolute path and exists */

     if (dir) {

@@ -1131,7 +1131,7 @@ virGetUserDirectoryByUID(uid_t uid ATTRIBUTE_UNUSED)

     if (!dir)

         /* USERPROFILE is probably the closest equivalent to $HOME? */

-        dir = getenv("USERPROFILE");

+        dir = virGetEnvBlockSUID("USERPROFILE");

     if (VIR_STRDUP(ret, dir) < 0)

         return NULL;

diff --git a/src/vbox/vbox_XPCOMCGlue.c b/src/vbox/vbox_XPCOMCGlue.c

index 10ddcf8..8652e3a 100644

--- a/src/vbox/vbox_XPCOMCGlue.c

+++ b/src/vbox/vbox_XPCOMCGlue.c

@@ -201,7 +201,7 @@ VBoxCGlueInit(unsigned int *version)

         "/usr/local/lib/VirtualBox",

         "/Applications/VirtualBox.app/Contents/MacOS"

     };

-    const char *home = getenv("VBOX_APP_HOME");

+    const char *home = virGetEnvBlockSUID("VBOX_APP_HOME");

     /* If the user specifies the location, try only that. */

     if (home != NULL) {

diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c

index 5e5ea85..2130590 100644

--- a/src/vbox/vbox_tmpl.c

+++ b/src/vbox/vbox_tmpl.c

@@ -2237,7 +2237,6 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) {

     vboxIID iid = VBOX_IID_INITIALIZER;

     int gotAllABoutDef   = -1;

     nsresult rc;

-    char *tmp;

     /* Flags checked by virDomainDefFormat */

@@ -2509,8 +2508,9 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) {

                     }

                 } else if ((vrdpPresent != 1) && (totalPresent == 0) && (VIR_ALLOC_N(def->graphics, 1) >= 0)) {

                     if (VIR_ALLOC(def->graphics[def->ngraphics]) >= 0) {

+                        const char *tmp;

                         def->graphics[def->ngraphics]->type = VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP;

-                        tmp = getenv("DISPLAY");

+                        tmp = virGetEnvBlockSUID("DISPLAY");

                         if (VIR_STRDUP(def->graphics[def->ngraphics]->data.desktop.display, tmp) < 0) {

                             /* just don't go to cleanup yet as it is ok to have

                              * display as NULL

diff --git a/tools/virsh.c b/tools/virsh.c

index d1ae183..48bf077 100644

--- a/tools/virsh.c

+++ b/tools/virsh.c

@@ -232,7 +232,7 @@ virshErrorHandler(void *unused ATTRIBUTE_UNUSED, virErrorPtr error)

 {

     virFreeError(last_error);

     last_error = virSaveLastError();

-    if (getenv("VIRSH_DEBUG") != NULL)

+    if (virGetEnvAllowSUID("VIRSH_DEBUG") != NULL)

         virDefaultErrorFunc(error);

 }

@@ -670,7 +670,7 @@ vshEditWriteToTempFile(vshControl *ctl, const char *doc)

     int fd;

     char ebuf[1024];

-    tmpdir = getenv("TMPDIR");

+    tmpdir = virGetEnvBlockSUID("TMPDIR");

     if (!tmpdir) tmpdir = "/tmp";

     if (virAsprintf(&ret, "%s/virshXXXXXX.xml", tmpdir) < 0) {

         vshError(ctl, "%s", _("out of memory"));

@@ -717,9 +717,9 @@ vshEditFile(vshControl *ctl, const char *filename)

     int outfd = STDOUT_FILENO;

     int errfd = STDERR_FILENO;

-    editor = getenv("VISUAL");

+    editor = virGetEnvBlockSUID("VISUAL");

     if (!editor)

-        editor = getenv("EDITOR");

+        editor = virGetEnvBlockSUID("EDITOR");

     if (!editor)

         editor = "vi"; /* could be cruel & default to ed(1) here */

@@ -2299,11 +2299,11 @@ vshEventLoop(void *opaque)

 static void

 vshInitDebug(vshControl *ctl)

 {

-    char *debugEnv;

+    const char *debugEnv;

     if (ctl->debug == VSH_DEBUG_DEFAULT) {

         /* log level not set from commandline, check env variable */

-        debugEnv = getenv("VIRSH_DEBUG");

+        debugEnv = virGetEnvAllowSUID("VIRSH_DEBUG");

         if (debugEnv) {

             int debug;

             if (virStrToLong_i(debugEnv, NULL, 10, &debug) < 0 ||

@@ -2318,7 +2318,7 @@ vshInitDebug(vshControl *ctl)

     if (ctl->logfile == NULL) {

         /* log file not set from cmdline */

-        debugEnv = getenv("VIRSH_LOG_FILE");

+        debugEnv = virGetEnvBlockSUID("VIRSH_LOG_FILE");

         if (debugEnv && *debugEnv) {

             ctl->logfile = vshStrdup(ctl, debugEnv);

             vshOpenLogFile(ctl);

@@ -3164,7 +3164,7 @@ int

 main(int argc, char **argv)

 {

     vshControl _ctl, *ctl = &_ctl;

-    char *defaultConn;

+    const char *defaultConn;

     bool ret = true;

     memset(ctl, 0, sizeof(vshControl));

@@ -3202,7 +3202,7 @@ main(int argc, char **argv)

     else

         progname++;

-    if ((defaultConn = getenv("VIRSH_DEFAULT_CONNECT_URI"))) {

+    if ((defaultConn = virGetEnvBlockSUID("VIRSH_DEFAULT_CONNECT_URI"))) {

         ctl->name = vshStrdup(ctl, defaultConn);

     }

-- 

1.8.4.2