From fcd9a141d08d521c01dc1a1c06a8d43a2337a392 Mon Sep 17 00:00:00 2001

From: Lennart Poettering <lennart@poettering.net>

Date: Mon, 4 Feb 2019 10:23:43 +0100

Subject: [PATCH] pam-systemd: use secure_getenv() rather than getenv()

And explain why in a comment.

(cherry picked from commit 83d4ab55336ff8a0643c6aa627b31e351a24040a)

CVE-2019-3842

Resolves: #1687514

---

 src/login/pam_systemd.c | 55 ++++++++++++++++++++++++-----------------

 1 file changed, 32 insertions(+), 23 deletions(-)

diff --git a/src/login/pam_systemd.c b/src/login/pam_systemd.c

index 1fbf6ba585..78ddb7d398 100644

--- a/src/login/pam_systemd.c

+++ b/src/login/pam_systemd.c

@@ -274,6 +274,33 @@ static int append_session_cg_weight(pam_handle_t *handle, sd_bus_message *m, con

         return 0;

 }

+static const char* getenv_harder(pam_handle_t *handle, const char *key, const char *fallback) {

+        const char *v;

+

+        assert(handle);

+        assert(key);

+

+        /* Looks for an environment variable, preferrably in the environment block associated with the

+         * specified PAM handle, falling back to the process' block instead. Why check both? Because we want

+         * to permit configuration of session properties from unit files that invoke PAM services, so that

+         * PAM services don't have to be reworked to set systemd-specific properties, but these properties

+         * can still be set from the unit file Environment= block. */

+

+        v = pam_getenv(handle, key);

+        if (!isempty(v))

+                return v;

+

+        /* We use secure_getenv() here, since we might get loaded into su/sudo, which are SUID. Ideally

+         * they'd clean up the environment before invoking foreign code (such as PAM modules), but alas they

+         * currently don't (to be precise, they clean up the environment they pass to their children, but

+         * not their own environ[]). */

+        v = secure_getenv(key);

+        if (!isempty(v))

+                return v;

+

+        return fallback;

+}

+

 _public_ PAM_EXTERN int pam_sm_open_session(

                 pam_handle_t *handle,

                 int flags,

@@ -352,29 +379,11 @@ _public_ PAM_EXTERN int pam_sm_open_session(

         pam_get_item(handle, PAM_RUSER, (const void**) &remote_user);

         pam_get_item(handle, PAM_RHOST, (const void**) &remote_host);

-        seat = pam_getenv(handle, "XDG_SEAT");

-        if (isempty(seat))

-                seat = getenv("XDG_SEAT");

-

-        cvtnr = pam_getenv(handle, "XDG_VTNR");

-        if (isempty(cvtnr))

-                cvtnr = getenv("XDG_VTNR");

-

-        type = pam_getenv(handle, "XDG_SESSION_TYPE");

-        if (isempty(type))

-                type = getenv("XDG_SESSION_TYPE");

-        if (isempty(type))

-                type = type_pam;

-

-        class = pam_getenv(handle, "XDG_SESSION_CLASS");

-        if (isempty(class))

-                class = getenv("XDG_SESSION_CLASS");

-        if (isempty(class))

-                class = class_pam;

-

-        desktop = pam_getenv(handle, "XDG_SESSION_DESKTOP");

-        if (isempty(desktop))

-                desktop = getenv("XDG_SESSION_DESKTOP");

+        seat = getenv_harder(handle, "XDG_SEAT", NULL);

+        cvtnr = getenv_harder(handle, "XDG_VTNR", NULL);

+        type = getenv_harder(handle, "XDG_SESSION_TYPE", type_pam);

+        class = getenv_harder(handle, "XDG_SESSION_CLASS", class_pam);

+        desktop = getenv_harder(handle, "XDG_SESSION_DESKTOP", NULL);

         tty = strempty(tty);