shared: in code that might get called from suid programs use __secure_getenv() rather...

It's better to be safe than sorry.

diff --git a/TODO b/TODO
index b1b57d66ff337fadf481e2786b7315e874a87e29..a4643d7b831e42b408b8ef5dd609512810f65501 100644 (file)
--- a/TODO
+++ b/TODO
@@ -65,8 +65,6 @@ Features:
 
 * maybe make systemd-detect-virt suid? or use fscaps?
 
-* consider using __secure_getenv() instead of getenv() in libs
-
 * man: document in ExecStart= explicitly that we don't take shell command lines, only executable names with arguments
 
 * shutdown: don't read-only mount anything when running in container
@@ -505,6 +503,8 @@ Regularly:
 
 * set_put(), hashmap_put() return values check. i.e. == 0 doesn't free()!
 
+* use __secure_getenv() instead of getenv() where appropriate
+
 Scheduled for removal (or fixing):
 
 * xxxOverridable dependencies

diff --git a/src/core/dbus.c b/src/core/dbus.c
index 9db172b6e692d5b6de52eb1d72bac9bb3530cac9..1fc714823e6b2538d93106053ae32b7a3414439d 100644 (file)
--- a/src/core/dbus.c
+++ b/src/core/dbus.c
@@ -955,12 +955,12 @@ static DBusConnection* manager_bus_connect_private(Manager *m, DBusBusType type)
 
         switch (type) {
         case DBUS_BUS_SYSTEM:
-                address = getenv("DBUS_SYSTEM_BUS_ADDRESS");
+                address = __secure_getenv("DBUS_SYSTEM_BUS_ADDRESS");
                 if (!address || !address[0])
                         address = DBUS_SYSTEM_BUS_DEFAULT_ADDRESS;
                 break;
         case DBUS_BUS_SESSION:
-                address = getenv("DBUS_SESSION_BUS_ADDRESS");
+                address = __secure_getenv("DBUS_SESSION_BUS_ADDRESS");
                 if (!address || !address[0])
                         address = DBUS_SESSION_BUS_DEFAULT_ADDRESS;
                 break;
@@ -1077,7 +1077,7 @@ static int bus_init_private(Manager *m) {
                 const char *e;
                 char *p;
 
-                e = getenv("XDG_RUNTIME_DIR");
+                e = __secure_getenv("XDG_RUNTIME_DIR");
                 if (!e)
                         return 0;
 

diff --git a/src/libudev/libudev.c b/src/libudev/libudev.c
index 07a24d5c3c61bdfc93a98801d50ff097f966f81c..1a74808411b58c1f5f6dd446994641d991b0758f 100644 (file)
--- a/src/libudev/libudev.c
+++ b/src/libudev/libudev.c
@@ -191,7 +191,7 @@ _public_ struct udev *udev_new(void)
         }
 
         /* environment overrides config */
-        env = getenv("UDEV_LOG");
+        env = __secure_getenv("UDEV_LOG");
         if (env != NULL)
                 udev_set_log_priority(udev, util_log_priority(env));
 

diff --git a/src/shared/dbus-common.c b/src/shared/dbus-common.c
index da2dc2e983a06c734616fc87b614ccef5617f986..8d7c4620cebac95f7f97e3858f99447ab9ef4a30 100644 (file)
--- a/src/shared/dbus-common.c
+++ b/src/shared/dbus-common.c
@@ -121,7 +121,7 @@ int bus_connect(DBusBusType t, DBusConnection **_bus, bool *_private, DBusError
                          * try via XDG_RUNTIME_DIR first, then
                          * fallback to normal bus access */
 
-                        e = getenv("XDG_RUNTIME_DIR");
+                        e = __secure_getenv("XDG_RUNTIME_DIR");
                         if (e) {
                                 char *p;
 

diff --git a/src/shared/log.c b/src/shared/log.c
index 1cbc9d625074f00d6f9b24e300e6145fdf2ec27e..4fc430eed11a1f783e9defeb89cf7317483def06 100644 (file)
--- a/src/shared/log.c
+++ b/src/shared/log.c
@@ -688,21 +688,21 @@ int log_set_max_level_from_string(const char *e) {
 void log_parse_environment(void) {
         const char *e;
 
-        if ((e = getenv("SYSTEMD_LOG_TARGET")))
-                if (log_set_target_from_string(e) < 0)
-                        log_warning("Failed to parse log target %s. Ignoring.", e);
+        e = __secure_getenv("SYSTEMD_LOG_TARGET");
+        if (e && log_set_target_from_string(e) < 0)
+                log_warning("Failed to parse log target %s. Ignoring.", e);
 
-        if ((e = getenv("SYSTEMD_LOG_LEVEL")))
-                if (log_set_max_level_from_string(e) < 0)
-                        log_warning("Failed to parse log level %s. Ignoring.", e);
+        e = __secure_getenv("SYSTEMD_LOG_LEVEL");
+        if (e && log_set_max_level_from_string(e) < 0)
+                log_warning("Failed to parse log level %s. Ignoring.", e);
 
-        if ((e = getenv("SYSTEMD_LOG_COLOR")))
-                if (log_show_color_from_string(e) < 0)
-                        log_warning("Failed to parse bool %s. Ignoring.", e);
+        e = __secure_getenv("SYSTEMD_LOG_COLOR");
+        if (e && log_show_color_from_string(e) < 0)
+                log_warning("Failed to parse bool %s. Ignoring.", e);
 
-        if ((e = getenv("SYSTEMD_LOG_LOCATION")))
-                if (log_show_location_from_string(e) < 0)
-                        log_warning("Failed to parse bool %s. Ignoring.", e);
+        e = __secure_getenv("SYSTEMD_LOG_LOCATION");
+        if (e && log_show_location_from_string(e) < 0)
+                log_warning("Failed to parse bool %s. Ignoring.", e);
 }
 
 LogTarget log_get_target(void) {