From: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Date: Wed, 3 Mar 2021 13:56:23 +0000 (+0100)
Subject: basic/escape: escape control characters, but not utf-8, in shell quoting
X-Git-Tag: v249-rc1~274^2~13
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=0089ab080036237b6e50afe99dea380fb2608f2c;p=thirdparty%2Fsystemd.git

basic/escape: escape control characters, but not utf-8, in shell quoting

The comment in the code said that so far this didn't matter, but I want to use
shell quoting in more places where this will make a difference. So control
characters are now escaped. Normal utf-8 characters are passed through, it
is 2021 after all and pretty much everyone is (or should be) using utf-8.

While touching the code, change 'char *r' → 'char *buf', in line with modern
style.
---

diff --git a/src/basic/escape.c b/src/basic/escape.c
index f45536d9bde..45899b6b7cc 100644
--- a/src/basic/escape.c
+++ b/src/basic/escape.c
@@ -465,62 +465,55 @@ char* octescape(const char *s, size_t len) {
 static char* strcpy_backslash_escaped(char *t, const char *s, const char *bad) {
         assert(bad);
 
-        for (; *s; s++) {
-                if (IN_SET(*s, '\n', '\t')) {
-                        *(t++) = '\\';
-                        *(t++) = *s == '\n' ? 'n' : 't';
-                        continue;
+        for (; *s; s++)
+                if (char_is_cc(*s))
+                        t += cescape_char(*s, t);
+                else {
+                        if (*s == '\\' || strchr(bad, *s))
+                                *(t++) = '\\';
+                        *(t++) = *s;
                 }
 
-                if (*s == '\\' || strchr(bad, *s))
-                        *(t++) = '\\';
-
-                *(t++) = *s;
-        }
-
         return t;
 }
 
 char* shell_escape(const char *s, const char *bad) {
-        char *r, *t;
+        char *buf, *t;
 
-        r = new(char, strlen(s)*2+1);
-        if (!r)
+        buf = new(char, strlen(s)*4+1);
+        if (!buf)
                 return NULL;
 
-        t = strcpy_backslash_escaped(r, s, bad);
+        t = strcpy_backslash_escaped(buf, s, bad);
         *t = 0;
 
-        return r;
+        return buf;
 }
 
 char* shell_maybe_quote(const char *s, ShellEscapeFlags flags) {
         const char *p;
-        char *r, *t;
+        char *buf, *t;
 
         assert(s);
 
-        /* Encloses a string in quotes if necessary to make it OK as a shell
-         * string. Note that we treat benign UTF-8 characters as needing
-         * escaping too, but that should be OK. */
+        /* Encloses a string in quotes if necessary to make it OK as a shell string. */
 
         if (FLAGS_SET(flags, SHELL_ESCAPE_EMPTY) && isempty(s))
                 return strdup("\"\""); /* We don't use $'' here in the POSIX mode. "" is fine too. */
 
         for (p = s; *p; p++)
-                if (*p <= ' ' ||
-                    *p >= 127 ||
-                    strchr(SHELL_NEED_QUOTES, *p))
+                if (char_is_cc(*p) ||
+                    strchr(WHITESPACE SHELL_NEED_QUOTES, *p))
                         break;
 
         if (!*p)
                 return strdup(s);
 
-        r = new(char, FLAGS_SET(flags, SHELL_ESCAPE_POSIX) + 1 + strlen(s)*2 + 1 + 1);
-        if (!r)
+        buf = new(char, FLAGS_SET(flags, SHELL_ESCAPE_POSIX) + 1 + strlen(s)*4 + 1 + 1);
+        if (!buf)
                 return NULL;
 
-        t = r;
+        t = buf;
         if (FLAGS_SET(flags, SHELL_ESCAPE_POSIX)) {
                 *(t++) = '$';
                 *(t++) = '\'';
@@ -538,5 +531,5 @@ char* shell_maybe_quote(const char *s, ShellEscapeFlags flags) {
                 *(t++) = '"';
         *t = 0;
 
-        return r;
+        return str_realloc(buf);
 }
diff --git a/src/test/test-escape.c b/src/test/test-escape.c
index 848aec37d8c..eca66ea9db3 100644
--- a/src/test/test-escape.c
+++ b/src/test/test-escape.c
@@ -118,6 +118,7 @@ static void test_shell_escape_one(const char *s, const char *bad, const char *ex
         _cleanup_free_ char *r;
 
         assert_se(r = shell_escape(s, bad));
+        log_debug("%s → %s (expected %s)", s, r, expected);
         assert_se(streq_ptr(r, expected));
 }
 
@@ -163,14 +164,22 @@ static void test_shell_maybe_quote(void) {
         test_shell_maybe_quote_one("foo$bar", SHELL_ESCAPE_POSIX, "$'foo$bar'");
         test_shell_maybe_quote_one("foo$bar", SHELL_ESCAPE_POSIX | SHELL_ESCAPE_EMPTY, "$'foo$bar'");
 
-        /* Note that current users disallow control characters, so this "test"
-         * is here merely to establish current behaviour. If control characters
-         * were allowed, they should be quoted, i.e. \001 should become \\001. */
-        test_shell_maybe_quote_one("a\nb\001", 0, "\"a\\nb\001\"");
-        test_shell_maybe_quote_one("a\nb\001", SHELL_ESCAPE_POSIX, "$'a\\nb\001'");
-
+        /* Exclamation mark is special in the interactive shell, but we don't treat it so. */
         test_shell_maybe_quote_one("foo!bar", 0, "\"foo!bar\"");
         test_shell_maybe_quote_one("foo!bar", SHELL_ESCAPE_POSIX, "$'foo!bar'");
+
+        /* Control characters and unicode */
+        test_shell_maybe_quote_one("a\nb\001", 0, "\"a\\nb\\001\"");
+        test_shell_maybe_quote_one("a\nb\001", SHELL_ESCAPE_POSIX, "$'a\\nb\\001'");
+
+        test_shell_maybe_quote_one("głąb", 0, "głąb");
+        test_shell_maybe_quote_one("głąb", SHELL_ESCAPE_POSIX, "głąb");
+
+        test_shell_maybe_quote_one("głąb\002\003", 0, "\"głąb\\002\\003\"");
+        test_shell_maybe_quote_one("głąb\002\003", SHELL_ESCAPE_POSIX, "$'głąb\\002\\003'");
+
+        test_shell_maybe_quote_one("głąb\002\003rząd", 0, "\"głąb\\002\\003rząd\"");
+        test_shell_maybe_quote_one("głąb\002\003rząd", SHELL_ESCAPE_POSIX, "$'głąb\\002\\003rząd'");
 }
 
 int main(int argc, char *argv[]) {