From 9a3dd0d5e8ccf3cc0867fee271acfe68e270e316 Mon Sep 17 00:00:00 2001 From: Liwei Ge Date: Wed, 28 Jun 2023 15:40:43 +0800 Subject: [PATCH 1/2] Make pager secure (CVE-2023-26604) Signed-off-by: Liwei Ge --- ...LESSSECURE-whenver-we-invoke-a-pager.patch | 111 ++++++++++++ ...er-secure-when-under-euid-is-changed.patch | 162 ++++++++++++++++++ systemd.spec | 7 +- 3 files changed, 279 insertions(+), 1 deletion(-) create mode 100644 10013-pager-set-LESSSECURE-whenver-we-invoke-a-pager.patch create mode 100644 10014-pager-make-pager-secure-when-under-euid-is-changed.patch diff --git a/10013-pager-set-LESSSECURE-whenver-we-invoke-a-pager.patch b/10013-pager-set-LESSSECURE-whenver-we-invoke-a-pager.patch new file mode 100644 index 0000000..094195b --- /dev/null +++ b/10013-pager-set-LESSSECURE-whenver-we-invoke-a-pager.patch @@ -0,0 +1,111 @@ +From 02dca3c62216002f8c1b15171d4f957a6f80458b Mon Sep 17 00:00:00 2001 +From: Liwei Ge +Date: Wed, 28 Jun 2023 15:31:39 +0800 +Subject: [PATCH 10013/10014] pager: set $LESSSECURE whenver we invoke a pager + +backport patch from +https://github.com/systemd/systemd/pull/16916/commits/ +9358eb98ee7ff3407dbcad010b53cfcc35a4060d + +Signed-off-by: Liwei Ge +--- + man/less-variables.xml | 8 ++++++++ + man/systemctl.xml | 1 + + man/systemd.xml | 2 ++ + src/basic/pager.c | 23 +++++++++++++++++++++-- + 4 files changed, 32 insertions(+), 2 deletions(-) + +diff --git a/man/less-variables.xml b/man/less-variables.xml +index a3faa38..9dad424 100644 +--- a/man/less-variables.xml ++++ b/man/less-variables.xml +@@ -36,5 +36,13 @@ + the invoking terminal is determined to be UTF-8 compatible). + + ++ ++ $SYSTEMD_LESSSECURE ++ ++ Takes a boolean argument. Overrides the $LESSSECURE environment ++ variable when invoking the pager, which controls the "secure" mode of less (which disables commands ++ such as | which allow to easily shell out to external command lines). By default ++ less secure mode is enabled, with this setting it may be disabled. ++ + + +diff --git a/man/systemctl.xml b/man/systemctl.xml +index a71e6c7..abc386e 100644 +--- a/man/systemctl.xml ++++ b/man/systemctl.xml +@@ -2010,6 +2010,7 @@ Jan 12 10:46:45 example.com bluetoothd[8900]: gatt-time-server: Input/output err + + + ++ + + + +diff --git a/man/systemd.xml b/man/systemd.xml +index 17ab59b..66ae4d8 100644 +--- a/man/systemd.xml ++++ b/man/systemd.xml +@@ -862,6 +862,8 @@ + + + ++ ++ + + $LISTEN_PID + $LISTEN_FDS +diff --git a/src/basic/pager.c b/src/basic/pager.c +index f241261..4efb01c 100644 +--- a/src/basic/pager.c ++++ b/src/basic/pager.c +@@ -11,6 +11,7 @@ + #include + + #include "copy.h" ++#include "env-util.h" + #include "fd-util.h" + #include "locale-util.h" + #include "log.h" +@@ -94,8 +95,7 @@ int pager_open(bool no_pager, bool jump_to_end) { + if (setenv("LESS", less_opts, 1) < 0) + _exit(EXIT_FAILURE); + +- /* Initialize a good charset for less. This is +- * particularly important if we output UTF-8 ++ /* Initialize a good charset for less. This is particularly important if we output UTF-8 + * characters. */ + less_charset = getenv("SYSTEMD_LESSCHARSET"); + if (!less_charset && is_locale_utf8()) +@@ -104,6 +104,25 @@ int pager_open(bool no_pager, bool jump_to_end) { + setenv("LESSCHARSET", less_charset, 1) < 0) + _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 */ ++ if (r < 0) ++ log_warning_errno(r, "Unable to parse $SYSTEMD_LESSSECURE, ignoring: %m"); ++ ++ if (setenv("LESSSECURE", "1", 1) < 0) { ++ log_error_errno(errno, "Failed to set environment variable LESSSECURE: %m"); ++ _exit(EXIT_FAILURE); ++ } ++ } ++ + if (pager) { + execlp(pager, pager, NULL); + execl("/bin/sh", "sh", "-c", pager, NULL); +-- +2.27.0 + diff --git a/10014-pager-make-pager-secure-when-under-euid-is-changed.patch b/10014-pager-make-pager-secure-when-under-euid-is-changed.patch new file mode 100644 index 0000000..0161d6b --- /dev/null +++ b/10014-pager-make-pager-secure-when-under-euid-is-changed.patch @@ -0,0 +1,162 @@ +From cb0335a867e8a9dfce737ed5f91f1e35bcb16af1 Mon Sep 17 00:00:00 2001 +From: Liwei Ge +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 +--- + 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 @@ + + + +- $SYSTEMD_LESSSECURE ++ $SYSTEMD_PAGERSECURE + +- Takes a boolean argument. Overrides the $LESSSECURE environment +- variable when invoking the pager, which controls the "secure" mode of less (which disables commands +- such as | which allow to easily shell out to external command lines). By default +- less secure mode is enabled, with this setting it may be disabled. ++ Takes a boolean argument. When true, the "secure" mode of the pager is enabled; if ++ false, disabled. If $SYSTEMD_PAGERSECURE 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 geteuid2 and ++ sd_pid_get_owner_uid3. ++ In secure mode, will be set when invoking the pager, and the pager shall ++ disable commands that open or create new files or start new subprocesses. When ++ $SYSTEMD_PAGERSECURE is not set at all, pagers which are not known to implement ++ secure mode will not be used. (Currently only ++ less1 implements ++ secure mode.) ++ ++ Note: when commands are invoked with elevated privileges, for example under sudo8 or ++ pkexec1, 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 SYSTEMD_PAGERSECURE=0 ++ or not removing it from the inherited environment allows the user to invoke arbitrary commands. Note ++ that if the $SYSTEMD_PAGER or $PAGER variables are to be ++ honoured, $SYSTEMD_PAGERSECURE must be set too. It might be reasonable to completly ++ disable the pager using instead. + + + +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 + #include + ++#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 + diff --git a/systemd.spec b/systemd.spec index 77fcc10..4e5c238 100644 --- a/systemd.spec +++ b/systemd.spec @@ -1,4 +1,4 @@ -%define anolis_release .0.1 +%define anolis_release .0.2 #global gitcommit 10e465b5321bd53c1fc59ffab27e724535c6bc0f %{?gitcommit:%global gitcommitshort %(c=%{gitcommit}; echo ${c:0:7})} @@ -969,6 +969,8 @@ Patch10009: 10009-systemd-anolis-support-loongarch64.patch Patch10010: 10010-test-catalog-Fix-coredump-when-compiled-under-GCC10.patch Patch10011: 10011-hwdb-add-Iluvatar-CoreX.patch Patch10012: 10012-seccomp-add-loongarch-support.patch +Patch10013: 10013-pager-set-LESSSECURE-whenver-we-invoke-a-pager.patch +Patch10014: 10014-pager-make-pager-secure-when-under-euid-is-changed.patch %ifarch %{ix86} x86_64 aarch64 %global have_gnu_efi 1 @@ -1599,6 +1601,9 @@ fi %files tests -f .file-list-tests %changelog +* Wed Jun 28 2023 Liwei Ge - 239-74.0.2 +- Make pager secure (CVE-2023-26604) + * Mon Jun 19 2023 Yuanhong Peng - 239-74.0.1 - core: fix a null reference case in load_from_path() - sysctl: Don't pass null directive argument to '%s' -- Gitee From cd13ca6d95f2e2dd41b9479891f569f36b29eb37 Mon Sep 17 00:00:00 2001 From: Liwei Ge Date: Wed, 28 Jun 2023 16:06:46 +0800 Subject: [PATCH 2/2] link libsystemd_static for sd_pid_get_owner_uid Signed-off-by: Liwei Ge --- ..._static-for-sd_pid_get_owner_uid-in-.patch | 26 +++++++++++++++++++ systemd.spec | 1 + 2 files changed, 27 insertions(+) create mode 100644 10015-link-libsystemd_static-for-sd_pid_get_owner_uid-in-.patch diff --git a/10015-link-libsystemd_static-for-sd_pid_get_owner_uid-in-.patch b/10015-link-libsystemd_static-for-sd_pid_get_owner_uid-in-.patch new file mode 100644 index 0000000..dedceff --- /dev/null +++ b/10015-link-libsystemd_static-for-sd_pid_get_owner_uid-in-.patch @@ -0,0 +1,26 @@ +From 46759790075ab6e474cb7f8160aac4e30261ed3b Mon Sep 17 00:00:00 2001 +From: Liwei Ge +Date: Wed, 28 Jun 2023 16:00:39 +0800 +Subject: [PATCH] link libsystemd_static for sd_pid_get_owner_uid in pager.c + +--- + meson.build | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +diff --git a/meson.build b/meson.build +index 673800a..d986dd2 100644 +--- a/meson.build ++++ b/meson.build +@@ -1467,7 +1467,8 @@ test_dlopen = executable( + 'test-dlopen', + test_dlopen_c, + include_directories : includes, +- link_with : [libbasic], ++ link_with : [libsystemd_static, ++ libbasic], + dependencies : [libdl]) + + foreach tuple : [['myhostname', 'ENABLE_NSS_MYHOSTNAME'], +-- +2.27.0 + diff --git a/systemd.spec b/systemd.spec index 4e5c238..38004ba 100644 --- a/systemd.spec +++ b/systemd.spec @@ -971,6 +971,7 @@ Patch10011: 10011-hwdb-add-Iluvatar-CoreX.patch Patch10012: 10012-seccomp-add-loongarch-support.patch Patch10013: 10013-pager-set-LESSSECURE-whenver-we-invoke-a-pager.patch Patch10014: 10014-pager-make-pager-secure-when-under-euid-is-changed.patch +Patch10015: 10015-link-libsystemd_static-for-sd_pid_get_owner_uid-in-.patch %ifarch %{ix86} x86_64 aarch64 %global have_gnu_efi 1 -- Gitee