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)
This commit is contained in:
Daniel P. Berrange 2013-10-09 11:18:15 +01:00
parent ef0476456a
commit 61fe5eebda
17 changed files with 41 additions and 37 deletions

View File

@ -991,7 +991,7 @@ static int migrateProfile(void)
goto cleanup; goto cleanup;
} }
config_home = getenv("XDG_CONFIG_HOME"); config_home = virGetEnvBlockSUID("XDG_CONFIG_HOME");
if (config_home && config_home[0] != '\0') { if (config_home && config_home[0] != '\0') {
if (VIR_STRDUP(xdg_dir, config_home) < 0) if (VIR_STRDUP(xdg_dir, config_home) < 0)
goto cleanup; goto cleanup;

View File

@ -27,6 +27,7 @@
#include "driver.h" #include "driver.h"
#include "viralloc.h" #include "viralloc.h"
#include "virlog.h" #include "virlog.h"
#include "virutil.h"
#include "configmake.h" #include "configmake.h"
#include "virstring.h" #include "virstring.h"
@ -43,7 +44,7 @@ static const char *moddir = NULL;
void void
virDriverModuleInitialize(const char *defmoddir) virDriverModuleInitialize(const char *defmoddir)
{ {
const char *custommoddir = getenv("LIBVIRT_DRIVER_DIR"); const char *custommoddir = virGetEnvBlockSUID("LIBVIRT_DRIVER_DIR");
if (custommoddir) if (custommoddir)
moddir = custommoddir; moddir = custommoddir;
else if (defmoddir) else if (defmoddir)

View File

@ -1094,7 +1094,7 @@ virConnectGetDefaultURI(virConfPtr conf,
{ {
int ret = -1; int ret = -1;
virConfValuePtr value = NULL; virConfValuePtr value = NULL;
char *defname = getenv("LIBVIRT_DEFAULT_URI"); const char *defname = virGetEnvBlockSUID("LIBVIRT_DEFAULT_URI");
if (defname && *defname) { if (defname && *defname) {
VIR_DEBUG("Using LIBVIRT_DEFAULT_URI '%s'", defname); VIR_DEBUG("Using LIBVIRT_DEFAULT_URI '%s'", defname);
*name = defname; *name = defname;

View File

@ -605,7 +605,7 @@ virLockDaemonSetupNetworkingSystemD(virNetServerPtr srv)
VIR_DEBUG("Setting up networking from systemd"); VIR_DEBUG("Setting up networking from systemd");
if (!(pidstr = getenv("LISTEN_PID"))) { if (!(pidstr = virGetEnvAllowSUID("LISTEN_PID"))) {
VIR_DEBUG("No LISTEN_FDS from systemd"); VIR_DEBUG("No LISTEN_FDS from systemd");
return 0; return 0;
} }
@ -621,7 +621,7 @@ virLockDaemonSetupNetworkingSystemD(virNetServerPtr srv)
return 0; return 0;
} }
if (!(fdstr = getenv("LISTEN_FDS"))) { if (!(fdstr = virGetEnvAllowSUID("LISTEN_FDS"))) {
VIR_DEBUG("No LISTEN_FDS from systemd"); VIR_DEBUG("No LISTEN_FDS from systemd");
return 0; return 0;
} }

View File

@ -84,7 +84,7 @@ static virLockManagerLockDaemonDriverPtr driver = NULL;
static const char * static const char *
virLockManagerLockDaemonFindDaemon(void) virLockManagerLockDaemonFindDaemon(void)
{ {
const char *customDaemon = getenv("VIRTLOCKD_PATH"); const char *customDaemon = virGetEnvBlockSUID("VIRTLOCKD_PATH");
if (customDaemon) if (customDaemon)
return customDaemon; return customDaemon;

View File

@ -135,7 +135,7 @@ virLockManagerPluginPtr virLockManagerPluginNew(const char *name,
void *handle = NULL; void *handle = NULL;
virLockDriverPtr driver; virLockDriverPtr driver;
virLockManagerPluginPtr plugin = NULL; 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 *modfile = NULL;
char *configFile = NULL; char *configFile = NULL;

View File

@ -1362,14 +1362,14 @@ static int lxcStateInitialize(bool privileged,
void *opaque ATTRIBUTE_UNUSED) void *opaque ATTRIBUTE_UNUSED)
{ {
virCapsPtr caps = NULL; virCapsPtr caps = NULL;
char *ld; const char *ld;
virLXCDriverConfigPtr cfg = NULL; virLXCDriverConfigPtr cfg = NULL;
/* Valgrind gets very annoyed when we clone containers, so /* Valgrind gets very annoyed when we clone containers, so
* disable LXC when under valgrind * disable LXC when under valgrind
* XXX remove this when valgrind is fixed * XXX remove this when valgrind is fixed
*/ */
ld = getenv("LD_PRELOAD"); ld = virGetEnvBlockSUID("LD_PRELOAD");
if (ld && strstr(ld, "vgpreload")) { if (ld && strstr(ld, "vgpreload")) {
VIR_INFO("Running under valgrind, disabling driver"); VIR_INFO("Running under valgrind, disabling driver");
return 0; return 0;

View File

@ -187,7 +187,7 @@ remoteFindDaemonPath(void)
NULL NULL
}; };
size_t i; size_t i;
const char *customDaemon = getenv("LIBVIRTD_PATH"); const char *customDaemon = virGetEnvBlockSUID("LIBVIRTD_PATH");
if (customDaemon) if (customDaemon)
return customDaemon; return customDaemon;
@ -955,7 +955,7 @@ remoteConnectOpen(virConnectPtr conn,
{ {
struct private_data *priv; struct private_data *priv;
int ret, rflags = 0; 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))) if (inside_daemon && (!conn->uri || (conn->uri && !conn->uri->server)))
return VIR_DRV_OPEN_DECLINED; return VIR_DRV_OPEN_DECLINED;

View File

@ -710,7 +710,7 @@ static virNetTLSContextPtr virNetTLSContextNew(const char *cacert,
bool isServer) bool isServer)
{ {
virNetTLSContextPtr ctxt; virNetTLSContextPtr ctxt;
char *gnutlsdebug; const char *gnutlsdebug;
int err; int err;
if (virNetTLSContextInitialize() < 0) if (virNetTLSContextInitialize() < 0)
@ -719,7 +719,7 @@ static virNetTLSContextPtr virNetTLSContextNew(const char *cacert,
if (!(ctxt = virObjectLockableNew(virNetTLSContextClass))) if (!(ctxt = virObjectLockableNew(virNetTLSContextClass)))
return NULL; return NULL;
if ((gnutlsdebug = getenv("LIBVIRT_GNUTLS_DEBUG")) != NULL) { if ((gnutlsdebug = virGetEnvAllowSUID("LIBVIRT_GNUTLS_DEBUG")) != NULL) {
int val; int val;
if (virStrToLong_i(gnutlsdebug, NULL, 10, &val) < 0) if (virStrToLong_i(gnutlsdebug, NULL, 10, &val) < 0)
val = 10; val = 10;

View File

@ -42,7 +42,7 @@ virAuthGetConfigFilePathURI(virURIPtr uri,
{ {
int ret = -1; int ret = -1;
size_t i; size_t i;
const char *authenv = getenv("LIBVIRT_AUTH_FILE"); const char *authenv = virGetEnvBlockSUID("LIBVIRT_AUTH_FILE");
char *userdir = NULL; char *userdir = NULL;
*path = NULL; *path = NULL;

View File

@ -1430,6 +1430,7 @@ virFileIsLink(const char *linkpath)
char * char *
virFindFileInPath(const char *file) virFindFileInPath(const char *file)
{ {
const char *origpath = NULL;
char *path = NULL; char *path = NULL;
char *pathiter; char *pathiter;
char *pathseg; char *pathseg;
@ -1458,9 +1459,11 @@ virFindFileInPath(const char *file)
} }
/* copy PATH env so we can tweak it */ /* 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; return NULL;
/* for each path segment, append the file to search for and test for /* for each path segment, append the file to search for and test for

View File

@ -1637,18 +1637,18 @@ virLogParseDefaultPriority(const char *priority)
void void
virLogSetFromEnv(void) virLogSetFromEnv(void)
{ {
char *debugEnv; const char *debugEnv;
if (virLogInitialize() < 0) if (virLogInitialize() < 0)
return; return;
debugEnv = getenv("LIBVIRT_DEBUG"); debugEnv = virGetEnvAllowSUID("LIBVIRT_DEBUG");
if (debugEnv && *debugEnv) if (debugEnv && *debugEnv)
virLogParseDefaultPriority(debugEnv); virLogParseDefaultPriority(debugEnv);
debugEnv = getenv("LIBVIRT_LOG_FILTERS"); debugEnv = virGetEnvAllowSUID("LIBVIRT_LOG_FILTERS");
if (debugEnv && *debugEnv) if (debugEnv && *debugEnv)
virLogParseFilters(debugEnv); virLogParseFilters(debugEnv);
debugEnv = getenv("LIBVIRT_LOG_OUTPUTS"); debugEnv = virGetEnvAllowSUID("LIBVIRT_LOG_OUTPUTS");
if (debugEnv && *debugEnv) if (debugEnv && *debugEnv)
virLogParseOutputs(debugEnv); virLogParseOutputs(debugEnv);
} }

View File

@ -66,7 +66,7 @@ virRandomOnceInit(void)
/* Normally we want a decent seed. But if reproducible debugging /* Normally we want a decent seed. But if reproducible debugging
* of a fixed pseudo-random sequence is ever required, uncomment * of a fixed pseudo-random sequence is ever required, uncomment
* this block to let an environment variable force the seed. */ * 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) if (debug && virStrToLong_ui(debug, NULL, 0, &seed) < 0)
return -1; return -1;

View File

@ -782,7 +782,7 @@ virGetUserDirectoryByUID(uid_t uid)
static char *virGetXDGDirectory(const char *xdgenvname, const char *xdgdefdir) static char *virGetXDGDirectory(const char *xdgenvname, const char *xdgdefdir)
{ {
const char *path = getenv(xdgenvname); const char *path = virGetEnvBlockSUID(xdgenvname);
char *ret = NULL; char *ret = NULL;
char *home = NULL; char *home = NULL;
@ -810,7 +810,7 @@ char *virGetUserCacheDirectory(void)
char *virGetUserRuntimeDirectory(void) char *virGetUserRuntimeDirectory(void)
{ {
const char *path = getenv("XDG_RUNTIME_DIR"); const char *path = virGetEnvBlockSUID("XDG_RUNTIME_DIR");
if (!path || !path[0]) { if (!path || !path[0]) {
return virGetUserCacheDirectory(); return virGetUserCacheDirectory();
@ -1143,7 +1143,7 @@ virGetUserDirectoryByUID(uid_t uid ATTRIBUTE_UNUSED)
const char *dir; const char *dir;
char *ret; char *ret;
dir = getenv("HOME"); dir = virGetEnvBlockSUID("HOME");
/* Only believe HOME if it is an absolute path and exists */ /* Only believe HOME if it is an absolute path and exists */
if (dir) { if (dir) {
@ -1163,7 +1163,7 @@ virGetUserDirectoryByUID(uid_t uid ATTRIBUTE_UNUSED)
if (!dir) if (!dir)
/* USERPROFILE is probably the closest equivalent to $HOME? */ /* USERPROFILE is probably the closest equivalent to $HOME? */
dir = getenv("USERPROFILE"); dir = virGetEnvBlockSUID("USERPROFILE");
if (VIR_STRDUP(ret, dir) < 0) if (VIR_STRDUP(ret, dir) < 0)
return NULL; return NULL;

View File

@ -201,7 +201,7 @@ VBoxCGlueInit(unsigned int *version)
"/usr/local/lib/VirtualBox", "/usr/local/lib/VirtualBox",
"/Applications/VirtualBox.app/Contents/MacOS" "/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 the user specifies the location, try only that. */
if (home != NULL) { if (home != NULL) {

View File

@ -2237,7 +2237,6 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) {
vboxIID iid = VBOX_IID_INITIALIZER; vboxIID iid = VBOX_IID_INITIALIZER;
int gotAllABoutDef = -1; int gotAllABoutDef = -1;
nsresult rc; nsresult rc;
char *tmp;
/* Flags checked by virDomainDefFormat */ /* 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)) { } else if ((vrdpPresent != 1) && (totalPresent == 0) && (VIR_ALLOC_N(def->graphics, 1) >= 0)) {
if (VIR_ALLOC(def->graphics[def->ngraphics]) >= 0) { if (VIR_ALLOC(def->graphics[def->ngraphics]) >= 0) {
const char *tmp;
def->graphics[def->ngraphics]->type = VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP; 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) { 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 /* just don't go to cleanup yet as it is ok to have
* display as NULL * display as NULL

View File

@ -233,7 +233,7 @@ virshErrorHandler(void *unused ATTRIBUTE_UNUSED, virErrorPtr error)
{ {
virFreeError(last_error); virFreeError(last_error);
last_error = virSaveLastError(); last_error = virSaveLastError();
if (getenv("VIRSH_DEBUG") != NULL) if (virGetEnvAllowSUID("VIRSH_DEBUG") != NULL)
virDefaultErrorFunc(error); virDefaultErrorFunc(error);
} }
@ -670,7 +670,7 @@ vshEditWriteToTempFile(vshControl *ctl, const char *doc)
int fd; int fd;
char ebuf[1024]; char ebuf[1024];
tmpdir = getenv("TMPDIR"); tmpdir = virGetEnvBlockSUID("TMPDIR");
if (!tmpdir) tmpdir = "/tmp"; if (!tmpdir) tmpdir = "/tmp";
if (virAsprintf(&ret, "%s/virshXXXXXX.xml", tmpdir) < 0) { if (virAsprintf(&ret, "%s/virshXXXXXX.xml", tmpdir) < 0) {
vshError(ctl, "%s", _("out of memory")); vshError(ctl, "%s", _("out of memory"));
@ -717,9 +717,9 @@ vshEditFile(vshControl *ctl, const char *filename)
int outfd = STDOUT_FILENO; int outfd = STDOUT_FILENO;
int errfd = STDERR_FILENO; int errfd = STDERR_FILENO;
editor = getenv("VISUAL"); editor = virGetEnvBlockSUID("VISUAL");
if (!editor) if (!editor)
editor = getenv("EDITOR"); editor = virGetEnvBlockSUID("EDITOR");
if (!editor) if (!editor)
editor = "vi"; /* could be cruel & default to ed(1) here */ editor = "vi"; /* could be cruel & default to ed(1) here */
@ -2367,11 +2367,11 @@ vshEventLoop(void *opaque)
static void static void
vshInitDebug(vshControl *ctl) vshInitDebug(vshControl *ctl)
{ {
char *debugEnv; const char *debugEnv;
if (ctl->debug == VSH_DEBUG_DEFAULT) { if (ctl->debug == VSH_DEBUG_DEFAULT) {
/* log level not set from commandline, check env variable */ /* log level not set from commandline, check env variable */
debugEnv = getenv("VIRSH_DEBUG"); debugEnv = virGetEnvAllowSUID("VIRSH_DEBUG");
if (debugEnv) { if (debugEnv) {
int debug; int debug;
if (virStrToLong_i(debugEnv, NULL, 10, &debug) < 0 || if (virStrToLong_i(debugEnv, NULL, 10, &debug) < 0 ||
@ -2386,7 +2386,7 @@ vshInitDebug(vshControl *ctl)
if (ctl->logfile == NULL) { if (ctl->logfile == NULL) {
/* log file not set from cmdline */ /* log file not set from cmdline */
debugEnv = getenv("VIRSH_LOG_FILE"); debugEnv = virGetEnvBlockSUID("VIRSH_LOG_FILE");
if (debugEnv && *debugEnv) { if (debugEnv && *debugEnv) {
ctl->logfile = vshStrdup(ctl, debugEnv); ctl->logfile = vshStrdup(ctl, debugEnv);
vshOpenLogFile(ctl); vshOpenLogFile(ctl);
@ -3232,7 +3232,7 @@ int
main(int argc, char **argv) main(int argc, char **argv)
{ {
vshControl _ctl, *ctl = &_ctl; vshControl _ctl, *ctl = &_ctl;
char *defaultConn; const char *defaultConn;
bool ret = true; bool ret = true;
memset(ctl, 0, sizeof(vshControl)); memset(ctl, 0, sizeof(vshControl));
@ -3279,7 +3279,7 @@ main(int argc, char **argv)
else else
progname++; progname++;
if ((defaultConn = getenv("VIRSH_DEFAULT_CONNECT_URI"))) { if ((defaultConn = virGetEnvBlockSUID("VIRSH_DEFAULT_CONNECT_URI"))) {
ctl->name = vshStrdup(ctl, defaultConn); ctl->name = vshStrdup(ctl, defaultConn);
} }