From cb0335a867e8a9dfce737ed5f91f1e35bcb16af1 Mon Sep 17 00:00:00 2001
From: Liwei Ge <liwei.glw@alibaba-inc.com>
Date: Wed, 28 Jun 2023 15:36:12 +0800
Subject: [PATCH 10014/10014] pager: make pager secure when under euid is
 changed

backport patch from
https://github.com/systemd/systemd/commit/
0a42426d797406b4b01a0d9c13bb759c2629d108

Signed-off-by: Liwei Ge <liwei.glw@alibaba-inc.com>
---
 man/less-variables.xml | 28 ++++++++++++++---
 src/basic/pager.c      | 69 +++++++++++++++++++++++++++---------------
 2 files changed, 67 insertions(+), 30 deletions(-)

diff --git a/man/less-variables.xml b/man/less-variables.xml
index 9dad424..5f3a53c 100644
--- a/man/less-variables.xml
+++ b/man/less-variables.xml
@@ -37,12 +37,30 @@
     </varlistentry>
 
     <varlistentry id='lesssecure'>
-      <term><varname>$SYSTEMD_LESSSECURE</varname></term>
+      <term><varname>$SYSTEMD_PAGERSECURE</varname></term>
 
-      <listitem><para>Takes a boolean argument. Overrides the <varname>$LESSSECURE</varname> environment
-      variable when invoking the pager, which controls the "secure" mode of less (which disables commands
-      such as <literal>|</literal> which allow to easily shell out to external command lines). By default
-      less secure mode is enabled, with this setting it may be disabled.</para></listitem>
+      <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>
diff --git a/src/basic/pager.c b/src/basic/pager.c
index 4efb01c..c7e1012 100644
--- a/src/basic/pager.c
+++ b/src/basic/pager.c
@@ -10,6 +10,8 @@
 #include <sys/prctl.h>
 #include <unistd.h>
 
+#include "sd-login.h"
+
 #include "copy.h"
 #include "env-util.h"
 #include "fd-util.h"
@@ -79,7 +81,7 @@ int pager_open(bool no_pager, bool jump_to_end) {
         if (r < 0)
                 return r;
         if (r == 0) {
-                const char* less_opts, *less_charset;
+                const char* less_opts, *less_charset, *exe;
 
                 /* In the child start the pager */
 
@@ -105,39 +107,56 @@ int pager_open(bool no_pager, bool jump_to_end) {
                         _exit(EXIT_FAILURE);
 
                 /* People might invoke us from sudo, don't needlessly allow less to be a way to shell out
-                 * privileged stuff. */
-                r = getenv_bool("SYSTEMD_LESSSECURE");
-                if (r == 0) { /* Remove env var if off */
-                        if (unsetenv("LESSSECURE") < 0) {
-                                log_error_errno(errno, "Failed to uset environment variable LESSSECURE: %m");
-                                _exit(EXIT_FAILURE);
-                        }
-                } else {
-                        /* Set env var otherwise */
+                 * 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. */
+                int use_secure_mode = getenv_bool("SYSTEMD_PAGERSECURE");
+                bool trust_pager = use_secure_mode >= 0;
+                if (use_secure_mode == -ENXIO) {
+                        uid_t uid;
+
+                        r = sd_pid_get_owner_uid(0, &uid);
                         if (r < 0)
-                                log_warning_errno(r, "Unable to parse $SYSTEMD_LESSSECURE, ignoring: %m");
+                                log_debug_errno(r, "sd_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;
+                }
 
-                        if (setenv("LESSSECURE", "1", 1) < 0) {
-                                log_error_errno(errno, "Failed to set environment variable LESSSECURE: %m");
-                                _exit(EXIT_FAILURE);
-                        }
+                /* 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 (pager) {
+                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 */
-- 
2.27.0