Skip to content

Commit

Permalink
pager: set $LESSSECURE whenver we invoke a pager
Browse files Browse the repository at this point in the history
Some extra safety when invoked via "sudo". With this we address a
genuine design flaw of sudo, and we shouldn't need to deal with this.
But it's still a good idea to disable this surface given how exotic it
is.

Prompted by systemd#5666
  • Loading branch information
poettering committed Sep 3, 2020
1 parent 88fdff4 commit efb5c55
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 2 deletions.
9 changes: 9 additions & 0 deletions man/less-variables.xml
Expand Up @@ -64,6 +64,15 @@
the invoking terminal is determined to be UTF-8 compatible).</para></listitem>
</varlistentry>

<varlistentry id='lesssecure'>
<term><varname>$SYSTEMD_LESSSECURE</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>
</varlistentry>

<varlistentry id='colors'>
<term><varname>$SYSTEMD_COLORS</varname></term>

Expand Down
1 change: 1 addition & 0 deletions man/systemctl.xml
Expand Up @@ -2240,6 +2240,7 @@ Jan 12 10:46:45 example.com bluetoothd[8900]: gatt-time-server: Input/output err
<xi:include href="less-variables.xml" xpointer="pager"/>
<xi:include href="less-variables.xml" xpointer="less"/>
<xi:include href="less-variables.xml" xpointer="lesscharset"/>
<xi:include href="less-variables.xml" xpointer="lesssecure"/>
<xi:include href="less-variables.xml" xpointer="colors"/>
<xi:include href="less-variables.xml" xpointer="urlify"/>
</refsect1>
Expand Down
1 change: 1 addition & 0 deletions man/systemd.xml
Expand Up @@ -692,6 +692,7 @@
<xi:include href="less-variables.xml" xpointer="pager"/>
<xi:include href="less-variables.xml" xpointer="less"/>
<xi:include href="less-variables.xml" xpointer="lesscharset"/>
<xi:include href="less-variables.xml" xpointer="lesssecure"/>
<xi:include href="less-variables.xml" xpointer="colors"/>
<xi:include href="less-variables.xml" xpointer="urlify"/>

Expand Down
24 changes: 22 additions & 2 deletions src/shared/pager.c
Expand Up @@ -9,6 +9,7 @@
#include <unistd.h>

#include "copy.h"
#include "env-util.h"
#include "fd-util.h"
#include "fileio.h"
#include "io-util.h"
Expand Down Expand Up @@ -152,8 +153,7 @@ int pager_open(PagerFlags flags) {
_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())
Expand All @@ -164,6 +164,26 @@ int pager_open(PagerFlags flags) {
_exit(EXIT_FAILURE);
}

/* People might invoke us from sudo, don't needlessly allow less to be a way to shell out
* privileged stuff. (Of course, this just illustrates the conceptual misdesign of "sudo",
* but still, it's an easy lockdown, so let's do it.) */
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_args) {
r = loop_write(exe_name_pipe[1], pager_args[0], strlen(pager_args[0]) + 1, false);
if (r < 0) {
Expand Down

0 comments on commit efb5c55

Please sign in to comment.