Remove all direct use of getenv

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)

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 99c0342afe9e1b027b4c7cd913f66a16934886c4..aef1546ad5cf566b687889ee45fae344253e4ba1 100644 (file)
--- 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 a08dd34cbe56c4589ddc9e8566c674fffedf2846..ab2a2538a0a69c4cc714b8f1e0e0af2ce73b9c09 100644 (file)
--- 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 7ceec30cfc2486c558596b3f5bcc25f14947d4c3..0f8d79a59be9c871769be085ae5df301ca848cdd 100644 (file)
--- 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 5f675ef7279fb0d2958a558ecf2b126717444a06..cef5bd66666fc97deed02fdb56bc46989a200ce5 100644 (file)
--- 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 b1157995f619ff3f15935647b7d2f93e97ea3c44..86ce2d87be9a78cc23e366a24e0e5e7f39f32ef0 100644 (file)
--- 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 aff54c0ee4ff0febe47b491a218f076ee2aac1bd..f09c168f5e266fabe6cfbaaedd8fe4bc1bb69152 100644 (file)
--- 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 e3a34d6dae689f4466113ce255e425ef659b5db6..ac952f20d121f0c96e1d948d92dfc2251e141c7c 100644 (file)
--- 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 0537e354b72b1f8d9d2c7bd290a74253f28c4747..43aaa82e8a9762870fc50e6249d4c3ba92741042 100644 (file)
--- 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 7cee27c8b7425678f8490960c707508623847ffe..cd69794a5aa69c8cb67d95332a9082a7a0701599 100644 (file)
--- 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 f58797c3700e7023f66b2c8da316da98047fb944..e66cbf5a38d392b39690148e2a163c1ac9d585fc 100644 (file)
--- 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 16f8101511a557b57738726587ca2f8998c57abf..0db3967ad67fae0a464411de7d64993996b24fcf 100644 (file)
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -1430,6 +1430,7 @@ virFileIsLink(const char *linkpath)
 char *
 virFindFileInPath(const char *file)
 {
+    const char *origpath = NULL;
     char *path = NULL;
     char *pathiter;
     char *pathseg;
@@ -1458,9 +1459,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 997d582c75b850b78d331648b71d8fe9cc15f782..5b7e7b06135fe0065af39a4c4c81d9fe02d3cf97 100644 (file)
--- 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 b0912f431c93b43dca9e201c2c0928e41c73fedf..491a3afc0ebb1658ec2c8332397f0442c9cf7cb2 100644 (file)
--- a/src/util/virrandom.c
+++ b/src/util/virrandom.c
@@ -66,7 +66,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 f0bba41c649a2c73ca6e73c4fa6f57662803e796..405006c3db1423b4334136eb6ceddde125c9839f 100644 (file)
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -782,7 +782,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;
 
@@ -810,7 +810,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();
@@ -1143,7 +1143,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) {
@@ -1163,7 +1163,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 10ddcf89ea2dd021ed3d84b75406849bb5ffc68f..8652e3a09074909e875c1b13dc86d38c12d9a5bd 100644 (file)
--- 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 5e5ea85456df57b1e62d6e1fd6b64d27c5c20532..213059085a8b99d34880d2a26186163f987a7f19 100644 (file)
--- 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 ac354acda55a5c680d0c3168882ba2503919f5d5..6cfd4172d462fbbf36e9cfa1a9860a95771aed06 100644 (file)
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -233,7 +233,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 */
 
@@ -2367,11 +2367,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 ||
@@ -2386,7 +2386,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);
@@ -3232,7 +3232,7 @@ int
 main(int argc, char **argv)
 {
     vshControl _ctl, *ctl = &_ctl;
-    char *defaultConn;
+    const char *defaultConn;
     bool ret = true;
 
     memset(ctl, 0, sizeof(vshControl));
@@ -3279,7 +3279,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);
     }