Skip to content

Commit 804ee07

Browse files
keszybzkeszybz
committed
Use "dollar-single-quotes" to escape shell-sensitive strings
Also called "ANSI-C Quoting" in info:(bash) ANSI-C Quoting. The escaping rules are a POSIX proposal, and are described in http://austingroupbugs.net/view.php?id=249. There's a lot of back-and-forth on the details of escaping of control characters, but we'll be only using a small subset of the syntax that is common to all proposals and is widely supported. Unfortunately dash and fish and maybe some other shells do not support it (see the man page patch for a list). This allows environment variables to be safely exported using show-environment and imported into the shell. Shells which do not support this syntax will have to do something like export $(systemctl show-environment|grep -v '=\$') or whatever is appropriate in their case. I think csh and fish do not support the A=B syntax anyway, so the change is moot for them. Fixes systemd#5536. v2: - also escape newlines (which currently disallowed in shell values, so this doesn't really matter), and tabs (as $'\t'), and ! (as $'!'). This way quoted output can be included directly in both interactive and noninteractive bash.
1 parent 42d3bf8 commit 804ee07

File tree

8 files changed

+135
-34
lines changed

8 files changed

+135
-34
lines changed

‎man/systemctl.xml‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1495,11 +1495,26 @@ Jan 12 10:46:45 example.com bluetoothd[8900]: gatt-time-server: Input/output err
14951495
<term><command>show-environment</command></term>
14961496

14971497
<listitem>
1498-
<para>Dump the systemd manager environment block. The
1499-
environment block will be dumped in straight-forward form
1500-
suitable for sourcing into a shell script. This environment
1501-
block will be passed to all processes the manager
1502-
spawns.</para>
1498+
<para>Dump the systemd manager environment block. This is the environment
1499+
block that is passed to all processes the manager spawns. The environment
1500+
block will be dumped in straight-forward form suitable for sourcing into
1501+
most shells. If no special characters or whitespace is present in the variable
1502+
values, no escaping is performed, and the assignments have the form
1503+
<literal>VARIABLE=value</literal>. If whitespace or characters which have
1504+
special meaning to the shell are present, dollar-single-quote escaping is
1505+
used, and assignments have the form <literal>VARIABLE=$'value'</literal>.
1506+
This syntax is known to be supported by
1507+
<citerefentry project='die-net'><refentrytitle>bash</refentrytitle><manvolnum>1</manvolnum></citerefentry>,
1508+
<citerefentry project='die-net'><refentrytitle>zsh</refentrytitle><manvolnum>1</manvolnum></citerefentry>,
1509+
<citerefentry project='die-net'><refentrytitle>ksh</refentrytitle><manvolnum>1</manvolnum></citerefentry>,
1510+
and
1511+
<citerefentry project='die-net'><refentrytitle>busybox</refentrytitle><manvolnum>1</manvolnum></citerefentry>'s
1512+
<citerefentry project='die-net'><refentrytitle>ash</refentrytitle><manvolnum>1</manvolnum></citerefentry>,
1513+
but not
1514+
<citerefentry project='die-net'><refentrytitle>dash</refentrytitle><manvolnum>1</manvolnum></citerefentry>
1515+
or
1516+
<citerefentry project='die-net'><refentrytitle>fish</refentrytitle><manvolnum>1</manvolnum></citerefentry>.
1517+
</para>
15031518
</listitem>
15041519
</varlistentry>
15051520
<varlistentry>

‎src/basic/escape.c‎

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -441,10 +441,16 @@ char *octescape(const char *s, size_t len) {
441441

442442
}
443443

444-
static char *strcpy_backslash_escaped(char *t, const char *s, const char *bad) {
444+
static char *strcpy_backslash_escaped(char *t, const char *s, const char *bad, bool escape_tab_nl) {
445445
assert(bad);
446446

447447
for (; *s; s++) {
448+
if (escape_tab_nl && IN_SET(*s, '\n', '\t')) {
449+
*(t++) = '\\';
450+
*(t++) = *s == '\n' ? 'n' : 't';
451+
continue;
452+
}
453+
448454
if (*s == '\\' || strchr(bad, *s))
449455
*(t++) = '\\';
450456

@@ -461,20 +467,21 @@ char *shell_escape(const char *s, const char *bad) {
461467
if (!r)
462468
return NULL;
463469

464-
t = strcpy_backslash_escaped(r, s, bad);
470+
t = strcpy_backslash_escaped(r, s, bad, false);
465471
*t = 0;
466472

467473
return r;
468474
}
469475

470-
char *shell_maybe_quote(const char *s) {
476+
char* shell_maybe_quote(const char *s, EscapeStyle style) {
471477
const char *p;
472478
char *r, *t;
473479

474480
assert(s);
475481

476-
/* Encloses a string in double quotes if necessary to make it
477-
* OK as shell string. */
482+
/* Encloses a string in quotes if necessary to make it OK as a shell
483+
* string. Note that we treat benign UTF-8 characters as needing
484+
* escaping too, but that should be OK. */
478485

479486
for (p = s; *p; p++)
480487
if (*p <= ' ' ||
@@ -485,17 +492,30 @@ char *shell_maybe_quote(const char *s) {
485492
if (!*p)
486493
return strdup(s);
487494

488-
r = new(char, 1+strlen(s)*2+1+1);
495+
r = new(char, (style == ESCAPE_POSIX) + 1 + strlen(s)*2 + 1 + 1);
489496
if (!r)
490497
return NULL;
491498

492499
t = r;
493-
*(t++) = '"';
500+
if (style == ESCAPE_BACKSLASH)
501+
*(t++) = '"';
502+
else if (style == ESCAPE_POSIX) {
503+
*(t++) = '$';
504+
*(t++) = '\'';
505+
} else
506+
assert_not_reached("Bad EscapeStyle");
507+
494508
t = mempcpy(t, s, p - s);
495509

496-
t = strcpy_backslash_escaped(t, p, SHELL_NEED_ESCAPE);
510+
if (style == ESCAPE_BACKSLASH)
511+
t = strcpy_backslash_escaped(t, p, SHELL_NEED_ESCAPE, false);
512+
else
513+
t = strcpy_backslash_escaped(t, p, SHELL_NEED_ESCAPE_POSIX, true);
497514

498-
*(t++)= '"';
515+
if (style == ESCAPE_BACKSLASH)
516+
*(t++) = '"';
517+
else
518+
*(t++) = '\'';
499519
*t = 0;
500520

501521
return r;

‎src/basic/escape.h‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,30 @@
3131
/* What characters are special in the shell? */
3232
/* must be escaped outside and inside double-quotes */
3333
#define SHELL_NEED_ESCAPE "\"\\`$"
34-
/* can be escaped or double-quoted */
35-
#define SHELL_NEED_QUOTES SHELL_NEED_ESCAPE GLOB_CHARS "'()<>|&;"
34+
35+
/* Those that can be escaped or double-quoted.
36+
*
37+
* Stricly speaking, ! does not need to be escaped, except in interactive
38+
* mode, but let's be extra nice to the user and quote ! in case this
39+
* output is ever used in interactive mode. */
40+
#define SHELL_NEED_QUOTES SHELL_NEED_ESCAPE GLOB_CHARS "'()<>|&;!"
41+
42+
/* Note that we assume control characters would need to be escaped too in
43+
* addition to the "special" characters listed here, if they appear in the
44+
* string. Current users disallow control characters. Also '"' shall not
45+
* be escaped.
46+
*/
47+
#define SHELL_NEED_ESCAPE_POSIX "\\\'"
3648

3749
typedef enum UnescapeFlags {
3850
UNESCAPE_RELAX = 1,
3951
} UnescapeFlags;
4052

53+
typedef enum EscapeStyle {
54+
ESCAPE_BACKSLASH = 1,
55+
ESCAPE_POSIX = 2,
56+
} EscapeStyle;
57+
4158
char *cescape(const char *s);
4259
char *cescape_length(const char *s, size_t n);
4360
size_t cescape_char(char c, char *buf);
@@ -51,4 +68,4 @@ char *xescape(const char *s, const char *bad);
5168
char *octescape(const char *s, size_t len);
5269

5370
char *shell_escape(const char *s, const char *bad);
54-
char *shell_maybe_quote(const char *s);
71+
char* shell_maybe_quote(const char *s, EscapeStyle style);

‎src/core/job.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -741,7 +741,7 @@ static void job_print_status_message(Unit *u, JobType t, JobResult result) {
741741
if (t == JOB_START && result == JOB_FAILED) {
742742
_cleanup_free_ char *quoted;
743743

744-
quoted = shell_maybe_quote(u->id);
744+
quoted = shell_maybe_quote(u->id, ESCAPE_BACKSLASH);
745745
manager_status_printf(u->manager, STATUS_TYPE_NORMAL, NULL, "See 'systemctl status %s' for details.", strna(quoted));
746746
}
747747
}

‎src/environment-d-generator/environment-d-generator.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ static int load_and_print(void) {
7878
t = strchr(*i, '=');
7979
assert(t);
8080

81-
q = shell_maybe_quote(t + 1);
81+
q = shell_maybe_quote(t + 1, ESCAPE_BACKSLASH);
8282
if (!q)
8383
return log_oom();
8484

‎src/shared/bus-unit-util.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -860,7 +860,7 @@ static void log_job_error_with_service_result(const char* service, const char *r
860860

861861
assert(service);
862862

863-
service_shell_quoted = shell_maybe_quote(service);
863+
service_shell_quoted = shell_maybe_quote(service, ESCAPE_BACKSLASH);
864864

865865
if (extra_args) {
866866
_cleanup_free_ char *t;

‎src/systemctl/systemctl.c‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
#include "dropin.h"
4848
#include "efivars.h"
4949
#include "env-util.h"
50+
#include "escape.h"
5051
#include "exit-status.h"
5152
#include "fd-util.h"
5253
#include "fileio.h"
@@ -5573,6 +5574,24 @@ static int reset_failed(int argc, char *argv[], void *userdata) {
55735574
return r;
55745575
}
55755576

5577+
static int print_variable(const char *s) {
5578+
const char *sep;
5579+
_cleanup_free_ char *esc = NULL;
5580+
5581+
sep = strchr(s, '=');
5582+
if (!sep) {
5583+
log_error("Invalid environment block");
5584+
return -EUCLEAN;
5585+
}
5586+
5587+
esc = shell_maybe_quote(sep + 1, ESCAPE_POSIX);
5588+
if (!esc)
5589+
return log_oom();
5590+
5591+
printf("%.*s=%s\n", (int)(sep-s), s, esc);
5592+
return 0;
5593+
}
5594+
55765595
static int show_environment(int argc, char *argv[], void *userdata) {
55775596
_cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
55785597
_cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
@@ -5602,8 +5621,11 @@ static int show_environment(int argc, char *argv[], void *userdata) {
56025621
if (r < 0)
56035622
return bus_log_parse_error(r);
56045623

5605-
while ((r = sd_bus_message_read_basic(reply, SD_BUS_TYPE_STRING, &text)) > 0)
5606-
puts(text);
5624+
while ((r = sd_bus_message_read_basic(reply, SD_BUS_TYPE_STRING, &text)) > 0) {
5625+
r = print_variable(text);
5626+
if (r < 0)
5627+
return r;
5628+
}
56075629
if (r < 0)
56085630
return bus_log_parse_error(r);
56095631

‎src/test/test-escape.c‎

Lines changed: 39 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -86,25 +86,52 @@ static void test_shell_escape(void) {
8686
test_shell_escape_one("foo:bar,baz", ",:", "foo\\:bar\\,baz");
8787
}
8888

89-
static void test_shell_maybe_quote_one(const char *s, const char *expected) {
90-
_cleanup_free_ char *r;
91-
92-
assert_se(r = shell_maybe_quote(s));
93-
assert_se(streq(r, expected));
89+
static void test_shell_maybe_quote_one(const char *s,
90+
EscapeStyle style,
91+
const char *expected) {
92+
_cleanup_free_ char *ret = NULL;
93+
94+
assert_se(ret = shell_maybe_quote(s, style));
95+
log_debug("[%s] → [%s] (%s)", s, ret, expected);
96+
assert_se(streq(ret, expected));
9497
}
9598

9699
static void test_shell_maybe_quote(void) {
97100

98-
test_shell_maybe_quote_one("", "");
99-
test_shell_maybe_quote_one("\\", "\"\\\\\"");
100-
test_shell_maybe_quote_one("\"", "\"\\\"\"");
101-
test_shell_maybe_quote_one("foobar", "foobar");
102-
test_shell_maybe_quote_one("foo bar", "\"foo bar\"");
103-
test_shell_maybe_quote_one("foo \"bar\" waldo", "\"foo \\\"bar\\\" waldo\"");
104-
test_shell_maybe_quote_one("foo$bar", "\"foo\\$bar\"");
101+
test_shell_maybe_quote_one("", ESCAPE_BACKSLASH, "");
102+
test_shell_maybe_quote_one("", ESCAPE_POSIX, "");
103+
test_shell_maybe_quote_one("\\", ESCAPE_BACKSLASH, "\"\\\\\"");
104+
test_shell_maybe_quote_one("\\", ESCAPE_POSIX, "$'\\\\'");
105+
test_shell_maybe_quote_one("\"", ESCAPE_BACKSLASH, "\"\\\"\"");
106+
test_shell_maybe_quote_one("\"", ESCAPE_POSIX, "$'\"'");
107+
test_shell_maybe_quote_one("foobar", ESCAPE_BACKSLASH, "foobar");
108+
test_shell_maybe_quote_one("foobar", ESCAPE_POSIX, "foobar");
109+
test_shell_maybe_quote_one("foo bar", ESCAPE_BACKSLASH, "\"foo bar\"");
110+
test_shell_maybe_quote_one("foo bar", ESCAPE_POSIX, "$'foo bar'");
111+
test_shell_maybe_quote_one("foo\tbar", ESCAPE_BACKSLASH, "\"foo\tbar\"");
112+
test_shell_maybe_quote_one("foo\tbar", ESCAPE_POSIX, "$'foo\\tbar'");
113+
test_shell_maybe_quote_one("foo\nbar", ESCAPE_BACKSLASH, "\"foo\nbar\"");
114+
test_shell_maybe_quote_one("foo\nbar", ESCAPE_POSIX, "$'foo\\nbar'");
115+
test_shell_maybe_quote_one("foo \"bar\" waldo", ESCAPE_BACKSLASH, "\"foo \\\"bar\\\" waldo\"");
116+
test_shell_maybe_quote_one("foo \"bar\" waldo", ESCAPE_POSIX, "$'foo \"bar\" waldo'");
117+
test_shell_maybe_quote_one("foo$bar", ESCAPE_BACKSLASH, "\"foo\\$bar\"");
118+
test_shell_maybe_quote_one("foo$bar", ESCAPE_POSIX, "$'foo$bar'");
119+
120+
/* Note that current users disallow control characters, so this "test"
121+
* is here merely to establish current behaviour. If control characters
122+
* were allowed, they should be quoted, i.e. \001 should become \\001. */
123+
test_shell_maybe_quote_one("a\nb\001", ESCAPE_BACKSLASH, "\"a\nb\001\"");
124+
test_shell_maybe_quote_one("a\nb\001", ESCAPE_POSIX, "$'a\\nb\001'");
125+
126+
test_shell_maybe_quote_one("foo!bar", ESCAPE_BACKSLASH, "\"foo!bar\"");
127+
test_shell_maybe_quote_one("foo!bar", ESCAPE_POSIX, "$'foo!bar'");
105128
}
106129

107130
int main(int argc, char *argv[]) {
131+
log_set_max_level(LOG_DEBUG);
132+
log_parse_environment();
133+
log_open();
134+
108135
test_cescape();
109136
test_cunescape();
110137
test_shell_escape();

0 commit comments

Comments
 (0)