From fcd9a141d08d521c01dc1a1c06a8d43a2337a392 Mon Sep 17 00:00:00 2001 From: Lennart Poettering 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);