(RHEL-35665) CVE-2023-26604 systemd: privilege escalation via the less pager

man/less-variables.xml

@@ -25,5 +25,33 @@
                         <command>less</command>
                         (<literal>FRSXMK</literal>).</para></listitem>
                 </varlistentry>
+
+                <varlistentry id='lesssecure'>
+                        <term><varname>$SYSTEMD_PAGERSECURE</varname></term>
+
+                        <listitem><para>Takes a boolean argument. When true, the "secure" mode of the pager is enabled; if
+                        false, disabled. If <varname>$SYSTEMD_PAGERSECURE</varname> is not set at all, secure mode is enabled
+                        if the effective UID is not the same as the owner of the login session, see <citerefentry
+                        project='man-pages'><refentrytitle>geteuid</refentrytitle><manvolnum>2</manvolnum></citerefentry> and
+                        <citerefentry><refentrytitle>sd_pid_get_owner_uid</refentrytitle><manvolnum>3</manvolnum></citerefentry>.
+                        In secure mode, <option>LESSSECURE=1</option> will be set when invoking the pager, and the pager shall
+                        disable commands that open or create new files or start new subprocesses. When
+                        <varname>$SYSTEMD_PAGERSECURE</varname> is not set at all, pagers which are not known to implement
+                        secure mode will not be used. (Currently only
+                        <citerefentry><refentrytitle>less</refentrytitle><manvolnum>1</manvolnum></citerefentry> implements
+                        secure mode.)</para>
+
+                        <para>Note: when commands are invoked with elevated privileges, for example under <citerefentry
+                        project='man-pages'><refentrytitle>sudo</refentrytitle><manvolnum>8</manvolnum></citerefentry> or
+                        <citerefentry
+                        project='die-net'><refentrytitle>pkexec</refentrytitle><manvolnum>1</manvolnum></citerefentry>, care
+                        must be taken to ensure that unintended interactive features are not enabled. "Secure" mode for the
+                        pager may be enabled automatically as describe above. Setting <varname>SYSTEMD_PAGERSECURE=0</varname>
+                        or not removing it from the inherited environment allows the user to invoke arbitrary commands. Note
+                        that if the <varname>$SYSTEMD_PAGER</varname> or <varname>$PAGER</varname> variables are to be
+                        honoured, <varname>$SYSTEMD_PAGERSECURE</varname> must be set too. It might be reasonable to completly
+                        disable the pager using <option>--no-pager</option> instead.</para></listitem>
+                </varlistentry>
+
         </variablelist>
 </refsect1>

man/systemctl.xml

@@ -1716,6 +1716,7 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service
     </variablelist>
     <xi:include href="less-variables.xml" xpointer="pager"/>
     <xi:include href="less-variables.xml" xpointer="less"/>
+    <xi:include href="less-variables.xml" xpointer="lesssecure"/>
   </refsect1>
 
   <refsect1>

man/systemd.xml

@@ -761,6 +761,8 @@
         </para></listitem>
       </varlistentry>
 
+      <xi:include href="less-variables.xml" xpointer="lesssecure"/>
+
       <varlistentry>
         <term><varname>$LISTEN_PID</varname></term>
         <term><varname>$LISTEN_FDS</varname></term>

src/shared/pager.c

@@ -26,9 +26,13 @@
 #include <string.h>
 #include <sys/prctl.h>
 
+#include "cgroup-util.h" /* cg_pid_get_owner_uid() */
+
+#include "env-util.h"
 #include "pager.h"
 #include "util.h"
 #include "macro.h"
+#include "strv.h"
 
 static pid_t pager_pid = 0;
 
@@ -47,6 +51,16 @@ noreturn static void pager_fallback(void) {
         _exit(EXIT_SUCCESS);
 }
 
+/* Custom wrapper replacing sd_pid_get_owner_uid() from sd-login.h
+ * added to avoid having to include sd-login.h that causes linking problems */
+static int pg_pid_get_owner_uid(pid_t pid, uid_t *uid) {
+
+        assert_return(pid >= 0, -EINVAL);
+        assert_return(uid, -EINVAL);
+
+        return cg_pid_get_owner_uid(pid, uid);
+}
+
 int pager_open(bool jump_to_end) {
         int fd[2];
         const char *pager;
@@ -82,7 +96,9 @@ int pager_open(bool jump_to_end) {
 
         /* In the child start the pager */
         if (pager_pid == 0) {
-                const char* less_opts;
+                const char* less_opts, *exe;
+                int use_secure_mode;
+                bool trust_pager;
 
                 dup2(fd[0], STDIN_FILENO);
                 safe_close_pair(fd);
@@ -103,21 +119,58 @@ int pager_open(bool jump_to_end) {
                 if (getppid() != parent_pid)
                         _exit(EXIT_SUCCESS);
 
-                if (pager) {
+                /* People might invoke us from sudo, don't needlessly allow less to be a way to shell out
+                 * privileged stuff. If the user set $SYSTEMD_PAGERSECURE, trust their configuration of the
+                 * pager. If they didn't, use secure mode when under euid is changed. If $SYSTEMD_PAGERSECURE
+                 * wasn't explicitly set, and we autodetect the need for secure mode, only use the pager we
+                 * know to be good. */
+                use_secure_mode = getenv_bool("SYSTEMD_PAGERSECURE");
+                trust_pager = use_secure_mode >= 0;
+
+                if (use_secure_mode == -ENXIO) {
+                        uid_t uid;
+
+                        r = pg_pid_get_owner_uid(0, &uid);
+                        if (r < 0)
+                                log_debug_errno(r, "pg_pid_get_owner_uid() failed, enabling pager secure mode: %m");
+
+                        use_secure_mode = r < 0 || uid != geteuid();
+
+                } else if (use_secure_mode < 0) {
+                        log_warning_errno(use_secure_mode, "Unable to parse $SYSTEMD_PAGERSECURE, assuming true: %m");
+                        use_secure_mode = true;
+                }
+
+                /* We generally always set variables used by less, even if we end up using a different pager.
+                 * They shouldn't hurt in any case, and ideally other pagers would look at them too. */
+                if (use_secure_mode)
+                        r = setenv("LESSSECURE", "1", 1);
+                else
+                        r = unsetenv("LESSSECURE");
+                if (r < 0) {
+                        log_error_errno(errno, "Failed to adjust environment variable LESSSECURE: %m");
+                        _exit(EXIT_FAILURE);
+                }
+
+                if (trust_pager && pager) { /* The pager config might be set globally, and we cannot
+                                                  * know if the user adjusted it to be appropriate for the
+                                                  * secure mode. Thus, start the pager specified through
+                                                  * envvars only when $SYSTEMD_PAGERSECURE was explicitly set
+                                                  * as well. */
                         execlp(pager, pager, NULL);
                         execl("/bin/sh", "sh", "-c", pager, NULL);
                 }
 
-                /* Debian's alternatives command for pagers is
-                 * called 'pager'. Note that we do not call
-                 * sensible-pagers here, since that is just a
-                 * shell script that implements a logic that
-                 * is similar to this one anyway, but is
-                 * Debian-specific. */
-                execlp("pager", "pager", NULL);
+                /* Debian's alternatives command for pagers is called 'pager'. Note that we do not call
+                 * sensible-pagers here, since that is just a shell script that implements a logic that is
+                 * similar to this one anyway, but is Debian-specific. */
+                FOREACH_STRING(exe, "pager", "less", "more") {
+                        /* Only less implements secure mode right now. */
+                        if (use_secure_mode && !streq(exe, "less"))
+                                continue;
 
-                execlp("less", "less", NULL);
-                execlp("more", "more", NULL);
+                        execlp(exe, exe, NULL);
+                }
 
                 pager_fallback();
                 /* not reached */