]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
string-util: beef up string_is_safe()
authorLennart Poettering <lennart@amutable.com>
Thu, 16 Apr 2026 07:03:24 +0000 (09:03 +0200)
committerLennart Poettering <lennart@poettering.net>
Tue, 21 Apr 2026 15:07:53 +0000 (17:07 +0200)
This tightens the checks of string_is_safe() and then adds flags to
relax certain aspects of it.

This does alter the rules on certain strings we pass a bit. We mostly
tighten the rules (but I think it's find and good) but we relax them on
others.

I let claude review the changes in behaviour for the various call sites
that I made. It summarized things in this table:

  ╭───────────────────────────────────────────────────┬──────────────────────────────────────────────╮
  │ CALL SITE                                         │ EFFECTIVE DELTA                              │
  ├───────────────────────────────────────────────────┼──────────────────────────────────────────────┤
  │ src/basic/syslog-util  log_namespace_name_valid   │ +UTF-8 required (globs already blocked)      │
  │ src/bootctl  --efi-boot-option-description        │ RELAXED: '\' and quotes now permitted        │
  │ src/core/dbus-manager  pretimeout governor        │ +UTF-8, +no-globs                            │
  │ src/core/load-fragment  ExecStart= path           │ +UTF-8, +no-globs                            │
  │ src/core/main  pretimeout governor (kcmdline)     │ +UTF-8, +no-globs                            │
  │ src/core/service  sd_notify STATUS=               │ +no-globs (ASCII-only preserved)             │
  │ src/home/homectl  --<identity field>=             │ empty now REJECTED; +UTF-8                   │
  │ src/libsystemd-network  dhcp_option_parse_string  │ (equivalent, just explicit)                  │
  │ src/libsystemd-network  sd_dhcp_server boot_fname │ ""→NULL coerced; else equivalent             │
  │ src/libsystemd/journal  SYSLOG_IDENTIFIER fb      │ +UTF-8, +no-globs                            │
  │ src/libsystemd/sd-json  SD_JSON_STRICT strings    │ +UTF-8 required                              │
  │ src/login/logind  session desktop=                │ +UTF-8 required                              │
  │ src/pcrlock  EFI variable string                  │ +UTF-8                                       │
  │ src/pcrlock  EFI action string                    │ RELAXED: empty + '\' now ok; +UTF-8          │
  │ src/resolve  dns-delegate id (from filename)      │ +UTF-8, +no-globs                            │
  │ src/shared/boot-entry  boot_entry_token_valid     │ (equivalent)                                 │
  │ src/shared/conf-parser  section header            │ +UTF-8, +no-globs                            │
  │ src/shared/conf-parser  CONFIG_PARSE_STRING_SAFE  │ +UTF-8 required                              │
  │ src/shared/kbd-util  keymap_is_valid              │ (equivalent; folded into STRING_FILENAME)    │
  │ src/shared/tpm2  nvpcr name                       │ +UTF-8 required                              │
  │ src/shared/vconsole  x11 layout/model/variant/opt │ +UTF-8, +no-globs                            │
  │ src/systemctl  --kernel-cmdline=                  │ +0x7f DEL rejected; empty path split out     │
  │ src/veritysetup  salt=                            │ RELAXED: safety check removed entirely       │
  │ src/vmspawn  --ssh-key-type=                      │ +UTF-8 required                              │
  ╰───────────────────────────────────────────────────┴──────────────────────────────────────────────╯

26 files changed:
src/basic/string-util.c
src/basic/string-util.h
src/basic/syslog-util.c
src/bootctl/bootctl.c
src/core/dbus-manager.c
src/core/load-fragment.c
src/core/main.c
src/core/manager.c
src/core/service.c
src/home/homectl.c
src/libsystemd-network/dhcp-option.c
src/libsystemd-network/sd-dhcp-server.c
src/libsystemd/sd-journal/journal-send.c
src/libsystemd/sd-json/sd-json.c
src/login/logind-dbus.c
src/pcrlock/pcrlock.c
src/resolve/resolved-dns-delegate.c
src/shared/boot-entry.c
src/shared/conf-parser.c
src/shared/kbd-util.c
src/shared/tpm2-util.c
src/shared/vconsole-util.c
src/systemctl/systemctl.c
src/test/test-string-util.c
src/veritysetup/veritysetup.c
src/vmspawn/vmspawn.c

index a2bbaf1ea8fbbc1c0f0b064bf01d213072a011fa..4d930839232d707545258ec4b99f99261c90cae7 100644 (file)
@@ -1099,25 +1099,40 @@ int strdup_to_full(char **ret, const char *src) {
         }
 };
 
-bool string_is_safe(const char *p) {
-        if (!p)
+bool string_is_safe(const char *p, StringSafeFlags flags) {
+
+        /* Baseline checks are:
+         *   • No control characters (i.e. 0…31 + 127)
+         *   • UTF-8 valid (well, technically we skip this test if STRING_ASCII is set, since that is a tighter test)
+         */
+
+        if (FLAGS_SET(flags, STRING_ALLOW_EMPTY) ? !p : isempty(p))
                 return false;
 
-        /* Checks if the specified string contains no quotes or control characters */
+        if (!FLAGS_SET(flags, STRING_ASCII) && !utf8_is_valid(p))
+                return false;
 
         for (const char *t = p; *t; t++) {
-                if (*t > 0 && *t < ' ') /* no control characters */
+                if ((*t > 0 && *t < ' ') || *t == 0x7f) /* never allow control characters */
+                        return false;
+
+                if (!FLAGS_SET(flags, STRING_ALLOW_BACKSLASHES) && *t == '\\')
                         return false;
 
-                if (strchr(QUOTES "\\\x7f", *t))
+                if (!FLAGS_SET(flags, STRING_ALLOW_QUOTES) && strchr(QUOTES, *t))
+                        return false;
+
+                if (!FLAGS_SET(flags, STRING_ALLOW_GLOBS) && strchr(GLOB_CHARS, *t))
+                        return false;
+
+                if (FLAGS_SET(flags, STRING_ASCII) && (uint8_t) *t >= 0x80)
                         return false;
         }
 
-        return true;
-}
+        if (FLAGS_SET(flags, STRING_FILENAME) && !filename_is_valid(p))
+                return false;
 
-bool string_is_safe_ascii(const char *p) {
-        return ascii_is_valid(p) && string_is_safe(p);
+        return true;
 }
 
 char* str_realloc(char *p) {
index 820b8688c22292217100f16f9658787b3877ae9f..17eaf5d9a6a8249985acca5b7d734516168a1d54 100644 (file)
@@ -220,8 +220,16 @@ static inline int strdup_to(char **ret, const char *src) {
         return r < 0 ? r : 0;  /* Suppress return value of 1. */
 }
 
-bool string_is_safe(const char *p) _pure_;
-bool string_is_safe_ascii(const char *p) _pure_;
+typedef enum StringSafeFlags {
+        STRING_ASCII             = 1 << 0, /* Verify string is 7-Bit ASCII (rather than just UTF-8) */
+        STRING_ALLOW_EMPTY       = 1 << 1, /* Allow empty strings */
+        STRING_ALLOW_BACKSLASHES = 1 << 2, /* Allow backslashes (\) */
+        STRING_ALLOW_QUOTES      = 1 << 3, /* Allow quotes (" or ') */
+        STRING_ALLOW_GLOBS       = 1 << 4, /* Allow globs (?, * or [) */
+        STRING_FILENAME          = 1 << 5, /* Verify the string is valid as regular filename */
+} StringSafeFlags;
+
+bool string_is_safe(const char *p, StringSafeFlags flags) _pure_;
 
 DISABLE_WARNING_STRINGOP_TRUNCATION;
 static inline void strncpy_exact(char *buf, const char *src, size_t buf_len) {
index fd910ca76d45084d6da934ad65e5150834e47950..bd1bb65282332a17a01d2ea9836e04ca2768f19e 100644 (file)
@@ -4,9 +4,7 @@
 
 #include "sd-id128.h"
 
-#include "glob-util.h"
 #include "hexdecoct.h"
-#include "path-util.h"
 #include "string-table.h"
 #include "string-util.h"
 #include "syslog-util.h"
@@ -111,7 +109,8 @@ bool log_namespace_name_valid(const char *s) {
          * (so that /var/log/journal/<machine-id>.<namespace> can be created based on it). Also make sure it
          * is suitable as unit instance name, and does not contain fishy characters. */
 
-        if (!filename_is_valid(s))
+        /* Let's avoid globbing for now */
+        if (!string_is_safe(s, STRING_FILENAME))
                 return false;
 
         if (strlen(s) > LOG_NAMESPACE_MAX)
@@ -120,12 +119,5 @@ bool log_namespace_name_valid(const char *s) {
         if (!unit_instance_is_valid(s))
                 return false;
 
-        if (!string_is_safe(s))
-                return false;
-
-        /* Let's avoid globbing for now */
-        if (string_is_glob(s))
-                return false;
-
         return true;
 }
index 308ff6a106d65dc7bc053c22385da06f5a7f2eff..00ae512668e7f567d2bbbd2de81f72246372dd11 100644 (file)
@@ -39,7 +39,6 @@
 #include "string-table.h"
 #include "string-util.h"
 #include "strv.h"
-#include "utf8.h"
 #include "varlink-io.systemd.BootControl.h"
 #include "varlink-util.h"
 #include "verbs.h"
@@ -573,7 +572,7 @@ static int parse_argv(int argc, char *argv[], char ***ret_args) {
 
                 OPTION_LONG("efi-boot-option-description", "DESCRIPTION",
                             "Description of the entry in the boot option list"):
-                        if (isempty(arg) || !(string_is_safe(arg) && utf8_is_valid(arg))) {
+                        if (!string_is_safe(arg, STRING_ALLOW_BACKSLASHES|STRING_ALLOW_QUOTES|STRING_ALLOW_GLOBS)) {
                                 _cleanup_free_ char *escaped = cescape(arg);
                                 return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
                                                        "Invalid --efi-boot-option-description=: %s", strna(escaped));
index 5a7f70d78bf6f7a742af4d0ab063c83f6a096ccf..1bc73e7b434c91a3d1af8e19296216c419c7e1a1 100644 (file)
@@ -387,13 +387,16 @@ static int property_set_pretimeout_watchdog_governor(
                 sd_bus_error *reterr_error) {
 
         Manager *m = ASSERT_PTR(userdata);
-        char *governor;
+        const char *governor;
         int r;
 
         r = sd_bus_message_read(value, "s", &governor);
         if (r < 0)
                 return r;
-        if (!string_is_safe(governor))
+
+        if (isempty(governor))
+                governor = NULL;
+        else if (!string_is_safe(governor, /* flags= */ 0))
                 return -EINVAL;
 
         return manager_override_watchdog_pretimeout_governor(m, governor);
index 52005c8c43600b6ed1b854cb17e9d99ae82386c3..1fae521333f79f31d36ba8bc522dc0b459b354fd 100644 (file)
@@ -990,7 +990,7 @@ int config_parse_exec(
                                            ignore ? ", ignoring" : "", rvalue);
                                 return ignore ? 0 : -ENOEXEC;
                         }
-                        if (!string_is_safe(path)) {
+                        if (!string_is_safe(path, /* flags= */ 0)) {
                                 log_syntax(unit, ignore ? LOG_WARNING : LOG_ERR, filename, line, 0,
                                            "Executable path contains special characters%s: %s",
                                            ignore ? ", ignoring" : "", path);
index b4022105e88b48db657a1a5ac09dbd79fab693fc..e89cb9da3d1f3496ab09782dc9ad44d9edafc386 100644 (file)
@@ -502,7 +502,7 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat
                         return 0;
                 }
 
-                if (!string_is_safe(value)) {
+                if (!string_is_safe(value, /* flags= */ 0)) {
                         log_warning("Watchdog pretimeout governor '%s' is not valid, ignoring.", value);
                         return 0;
                 }
index 01841a97d6d226c4af53c8cfd3f651b33d1bae51..56ee66e5bd2928ec031931985f10d5d16ec06f27 100644 (file)
@@ -3576,12 +3576,14 @@ int manager_set_watchdog_pretimeout_governor(Manager *m, const char *governor) {
         if (MANAGER_IS_USER(m))
                 return 0;
 
+        governor = empty_to_null(governor);
+
         if (streq_ptr(m->watchdog_pretimeout_governor, governor))
                 return 0;
 
-        p = strdup(governor);
-        if (!p)
-                return -ENOMEM;
+        r = strdup_to(&p, governor);
+        if (r < 0)
+                return r;
 
         r = watchdog_setup_pretimeout_governor(governor);
         if (r < 0)
@@ -3599,12 +3601,14 @@ int manager_override_watchdog_pretimeout_governor(Manager *m, const char *govern
         if (MANAGER_IS_USER(m))
                 return 0;
 
+        governor = empty_to_null(governor);
+
         if (streq_ptr(m->watchdog_pretimeout_governor_overridden, governor))
                 return 0;
 
-        p = strdup(governor);
-        if (!p)
-                return -ENOMEM;
+        r = strdup_to(&p, governor);
+        if (r < 0)
+                return r;
 
         r = watchdog_setup_pretimeout_governor(governor);
         if (r < 0)
index 63e659942188f36ac8a30e632a47b2c1b99ae18f..b2a656a6fe84d5ed67382a06d1b8a209460fcf10 100644 (file)
@@ -5210,7 +5210,7 @@ static void service_notify_message(
 
                 e = empty_to_null(e);
 
-                if (e && !string_is_safe_ascii(e)) {
+                if (e && !string_is_safe(e, STRING_ASCII)) {
                         _cleanup_free_ char *escaped = cescape(e);
                         log_unit_warning(u, "Got invalid %s string, ignoring: %s", i->tag, strna(escaped));
                 } else if (free_and_strdup_warn(status_error, e) > 0)
index 194e73b4917497fa22e5f0ec6af1c163337a0043..4ebf47ca9e75a1ca486e456d8e5366de1e5e9208 100644 (file)
@@ -4666,7 +4666,7 @@ static int parse_argv(int argc, char *argv[]) {
                                 IN_SET(c, ARG_STORAGE, ARG_FS_TYPE) ?
                                 &arg_identity_extra_this_machine : &arg_identity_extra;
 
-                        if (!isempty(optarg) && !string_is_safe(optarg))
+                        if (!string_is_safe(optarg, STRING_ALLOW_GLOBS))
                                 return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
                                                        "Parameter for field %s not valid: %s", field, optarg);
 
index c78e3cdad9d7258e3f1e3021c187e9ca901b1c75..a195a4b0983538e4c48c0888fe5a57b1536f69d7 100644 (file)
@@ -427,7 +427,7 @@ int dhcp_option_parse_string(const uint8_t *option, size_t len, char **ret) {
         if (r < 0)
                 return r;
 
-        if (!string_is_safe(string) || !utf8_is_valid(string))
+        if (!string_is_safe(string, STRING_ALLOW_EMPTY|STRING_ALLOW_GLOBS))
                 return -EINVAL;
 
         *ret = TAKE_PTR(string);
index 34ed3e10d33bc7d2dde5cc4495d95133d89b5307..fa0b83019698322c2487d62e44ad7efb3508e50e 100644 (file)
@@ -271,7 +271,9 @@ int sd_dhcp_server_set_boot_server_name(sd_dhcp_server *server, const char *name
 int sd_dhcp_server_set_boot_filename(sd_dhcp_server *server, const char *filename) {
         assert_return(server, -EINVAL);
 
-        if (filename && !string_is_safe_ascii(filename))
+        if (isempty(filename))
+                filename = NULL;
+        else if (!string_is_safe(filename, STRING_ASCII|STRING_ALLOW_GLOBS))
                 return -EINVAL;
 
         return free_and_strdup(&server->boot_filename, filename);
index 5c7b007b131b2b45e75ebac2da7b48e25bdc857a..931e669a08926e04dcfbb95ead5e0ae177a090ab 100644 (file)
@@ -273,13 +273,11 @@ _public_ int sd_journal_sendv(const struct iovec *iov, int n) {
         }
 
         if (!have_syslog_identifier &&
-            string_is_safe(program_invocation_short_name)) {
+            string_is_safe(program_invocation_short_name, /* flags= */ 0)) {
 
-                /* Implicitly add program_invocation_short_name, if it
-                 * is not set explicitly. We only do this for
-                 * program_invocation_short_name, and nothing else
-                 * since everything else is much nicer to retrieve
-                 * from the outside. */
+                /* Implicitly add program_invocation_short_name, if it is not set explicitly. We only do this
+                 * for program_invocation_short_name, and nothing else since everything else is much nicer to
+                 * retrieve from the outside. */
 
                 w[j++] = IOVEC_MAKE_STRING("SYSLOG_IDENTIFIER=");
                 w[j++] = IOVEC_MAKE_STRING(program_invocation_short_name);
index 4c39b5660a649ad1fd0ee9f9e630cc3854869611..4c541275c42c5d7b549c9878622775263f886d5d 100644 (file)
@@ -5652,7 +5652,7 @@ _public_ int sd_json_dispatch_const_string(const char *name, sd_json_variant *va
         if (!sd_json_variant_is_string(variant))
                 return json_log(variant, flags, SYNTHETIC_ERRNO(EINVAL), "JSON field '%s' is not a string.", strna(name));
 
-        if ((flags & SD_JSON_STRICT) && !string_is_safe(sd_json_variant_string(variant)))
+        if ((flags & SD_JSON_STRICT) && !string_is_safe(sd_json_variant_string(variant), STRING_ALLOW_EMPTY|STRING_ALLOW_GLOBS))
                 return json_log(variant, flags, SYNTHETIC_ERRNO(EINVAL), "JSON field '%s' contains unsafe characters, refusing.", strna(name));
 
         *s = sd_json_variant_string(variant);
@@ -5675,7 +5675,7 @@ _public_ int sd_json_dispatch_strv(const char *name, sd_json_variant *variant, s
 
         /* Let's be flexible here: accept a single string in place of a single-item array */
         if (sd_json_variant_is_string(variant)) {
-                if ((flags & SD_JSON_STRICT) && !string_is_safe(sd_json_variant_string(variant)))
+                if ((flags & SD_JSON_STRICT) && !string_is_safe(sd_json_variant_string(variant), STRING_ALLOW_EMPTY|STRING_ALLOW_GLOBS))
                         return json_log(variant, flags, SYNTHETIC_ERRNO(EINVAL), "JSON field '%s' contains unsafe characters, refusing.", strna(name));
 
                 l = strv_new(sd_json_variant_string(variant));
@@ -5693,7 +5693,7 @@ _public_ int sd_json_dispatch_strv(const char *name, sd_json_variant *variant, s
                 if (!sd_json_variant_is_string(e))
                         return json_log(e, flags, SYNTHETIC_ERRNO(EINVAL), "JSON array element is not a string.");
 
-                if ((flags & SD_JSON_STRICT) && !string_is_safe(sd_json_variant_string(e)))
+                if ((flags & SD_JSON_STRICT) && !string_is_safe(sd_json_variant_string(e), STRING_ALLOW_EMPTY|STRING_ALLOW_GLOBS))
                         return json_log(e, flags, SYNTHETIC_ERRNO(EINVAL), "JSON field '%s' contains unsafe characters, refusing.", strna(name));
 
                 r = strv_extend(&l, sd_json_variant_string(e));
index 9668bf0609868c18c1254c3e9208e7caae927cb2..14208bc1496e9c967e40a3b38cdc45276b06c520 100644 (file)
@@ -1155,7 +1155,7 @@ static int manager_create_session_by_bus(
         if (isempty(desktop))
                 desktop = NULL;
         else {
-                if (!string_is_safe(desktop))
+                if (!string_is_safe(desktop, STRING_ALLOW_GLOBS))
                         return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS,
                                                  "Invalid desktop string %s", desktop);
         }
index a6ec5c23d03a2c4a971733a9fefeddf9db06ccf4..fd8b9e95a4fd098371a60e9f7f5c63fb7a60009e 100644 (file)
@@ -543,7 +543,7 @@ static int event_log_record_parse_variable_data(
         if (!p)
                 return log_oom_debug();
 
-        if (!string_is_safe(p))
+        if (!string_is_safe(p, STRING_ALLOW_GLOBS))
                 return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "Unsafe EFI variable string in record.");
 
         *ret_variable_uuid = efi_guid_to_id128(vdata->variableName);
@@ -627,7 +627,7 @@ static int event_log_record_extract_firmware_description(EventLogRecord *rec) {
                 if (r < 0)
                         return log_error_errno(r, "Failed to make C string from EFI action string: %m");
 
-                if (!string_is_safe(d)) {
+                if (!string_is_safe(d, STRING_ALLOW_GLOBS|STRING_ALLOW_EMPTY|STRING_ALLOW_BACKSLASHES)) {
                         log_warning("Unsafe EFI action string in record, ignoring.");
                         goto invalid;
                 }
index eee2daab94b94557c28964b6a3e0cb97e1d64fec..db34916a297716e7b762b4a74e31379933b4a8b6 100644 (file)
@@ -172,8 +172,8 @@ static int dns_delegate_load(Manager *m, const char *path) {
                 return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "DNS delegate file name does not end in .dns-delegate, refusing: %s", fn);
 
         _cleanup_free_ char *id = strndup(fn, e - fn);
-        if (!string_is_safe(id))
-                return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "DNS delegate file name contains weird characters, refusing: %s", fn);
+        if (!string_is_safe(id, /* flags= */ 0))
+                return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "DNS delegate file name is invalid, refusing: %s", fn);
 
         _cleanup_free_ char *dropin_dirname = strjoin(id, ".dns-delegate.d");
         if (!dropin_dirname)
index c9e966ba04ea883bfea6a930dc02368e5f32b36e..b0dac0d7825395ffca44eaf2ffba62a54d405545 100644 (file)
 #include "string-table.h"
 #include "string-util.h"
 #include "strv.h"
-#include "utf8.h"
 
 bool boot_entry_token_valid(const char *p) {
-        return utf8_is_valid(p) && string_is_safe(p) && filename_is_valid(p);
+        return string_is_safe(p, STRING_FILENAME);
 }
 
 static int entry_token_load_one(int rfd, const char *dir, BootEntryTokenType *type, char **token) {
index b448032939d5359e75b435b14c72759862d126d4..feaed66cbe264d03fa92d15ad2884317137915d6 100644 (file)
@@ -217,8 +217,8 @@ static int parse_line(
                 if (!n)
                         return log_oom();
 
-                if (!string_is_safe(n))
-                        return log_syntax(unit, LOG_ERR, filename, line, SYNTHETIC_ERRNO(EBADMSG), "Bad characters in section header '%s'", l);
+                if (!string_is_safe(n, /* flags= */ 0))
+                        return log_syntax(unit, LOG_ERR, filename, line, SYNTHETIC_ERRNO(EBADMSG), "Section header invalid '%s'", l);
 
                 if (sections && !nulstr_contains(sections, n)) {
                         bool ignore;
@@ -1244,7 +1244,7 @@ int config_parse_string(
                 return 1;
         }
 
-        if (FLAGS_SET(ltype, CONFIG_PARSE_STRING_SAFE) && !string_is_safe(rvalue)) {
+        if (FLAGS_SET(ltype, CONFIG_PARSE_STRING_SAFE) && !string_is_safe(rvalue, STRING_ALLOW_GLOBS)) {
                 _cleanup_free_ char *escaped = NULL;
 
                 escaped = cescape(rvalue);
index 84031df78435431cabf9c7ad1157a78de595778d..28ea2e8612e5de904554fb839a2a008d59269bdd 100644 (file)
@@ -5,12 +5,10 @@
 #include "errno-util.h"
 #include "kbd-util.h"
 #include "log.h"
-#include "path-util.h"
 #include "recurse-dir.h"
 #include "set.h"
 #include "string-util.h"
 #include "strv.h"
-#include "utf8.h"
 
 #define KBD_KEYMAP_DIRS                         \
         "/usr/share/keymaps/",                  \
@@ -129,21 +127,12 @@ int get_keymaps(char ***ret) {
 }
 
 bool keymap_is_valid(const char *name) {
-        if (isempty(name))
+        if (!string_is_safe(name, STRING_FILENAME))
                 return false;
 
         if (strlen(name) >= 128)
                 return false;
 
-        if (!utf8_is_valid(name))
-                return false;
-
-        if (!filename_is_valid(name))
-                return false;
-
-        if (!string_is_safe(name))
-                return false;
-
         return true;
 }
 
index 02f89c02ba71f2fde421906c861fac5e75e4e4cd..fa0fa71130b572d294eab760eeb5f88287f796da 100644 (file)
@@ -9489,7 +9489,6 @@ DEFINE_STRING_TABLE_LOOKUP_FROM_STRING_WITH_FALLBACK(tpm2_pcr_index, int, TPM2_P
 DEFINE_STRING_TABLE_LOOKUP_TO_STRING(tpm2_pcr_index, int);
 
 bool tpm2_nvpcr_name_is_valid(const char *name) {
-        return filename_is_valid(name) &&
-                string_is_safe(name) &&
+        return string_is_safe(name, STRING_FILENAME) &&
                 tpm2_pcr_index_from_string(name) < 0; /* don't allow nvpcrs to be name like pcrs */
 }
index cd623bebdbbb6282db0af75407353b351c654a98..6e8c17561e8a7f460c9bf57c11f3b55fa9c7419a 100644 (file)
@@ -87,10 +87,10 @@ bool x11_context_is_safe(const X11Context *xc) {
         assert(xc);
 
         return
-                (!xc->layout  || string_is_safe(xc->layout))  &&
-                (!xc->model   || string_is_safe(xc->model))   &&
-                (!xc->variant || string_is_safe(xc->variant)) &&
-                (!xc->options || string_is_safe(xc->options));
+                (!xc->layout  || string_is_safe(xc->layout, /* flags= */ 0))  &&
+                (!xc->model   || string_is_safe(xc->model, /* flags= */ 0))   &&
+                (!xc->variant || string_is_safe(xc->variant, /* flags= */ 0)) &&
+                (!xc->options || string_is_safe(xc->options, /* flags= */ 0));
 }
 
 bool x11_context_equal(const X11Context *a, const X11Context *b) {
index 5c9a24b119aded7e93037f81b21203d3eb84e399..e5e7b412f568d90c8d46f4c8adeff48c67fc8571 100644 (file)
@@ -27,7 +27,6 @@
 #include "systemctl-compat-shutdown.h"
 #include "systemctl-logind.h"
 #include "time-util.h"
-#include "utf8.h"
 
 char **arg_types = NULL;
 char **arg_states = NULL;
@@ -969,16 +968,18 @@ static int systemctl_parse_argv(int argc, char *argv[]) {
                         break;
 
                 case ARG_KERNEL_CMDLINE:
-                        if (!utf8_is_valid(optarg))
-                                return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
-                                                       "--kernel-cmdline= argument is not valid UTF-8: %s", optarg);
-                        if (string_has_cc(optarg, NULL))
+                        if (isempty(optarg)) {
+                                arg_kernel_cmdline = mfree(arg_kernel_cmdline);
+                                break;
+                        }
+
+                        if (!string_is_safe(optarg, STRING_ALLOW_GLOBS|STRING_ALLOW_BACKSLASHES|STRING_ALLOW_QUOTES))
                                 return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
-                                                       "--kernel-cmdline= argument contains control characters: %s", optarg);
+                                                       "--kernel-cmdline= argument contains invalid characters: %s", optarg);
 
                         r = free_and_strdup_warn(&arg_kernel_cmdline, optarg);
                         if (r < 0)
-                                 return r;
+                                return r;
                         break;
 
                 case ARG_TIMESTAMP_STYLE:
index 5707569e7e17d7e925eafd07175022fe8f3d7223..6e07d727636eef03c32e378e9996a337089e4fec 100644 (file)
@@ -1495,4 +1495,146 @@ TEST(str_common_prefix) {
         ASSERT_EQ(str_common_prefix("systemd-networkd", ""), 0U);
 }
 
+TEST(string_is_safe) {
+        /* NULL is always rejected, regardless of flags. */
+        ASSERT_FALSE(string_is_safe(NULL, 0));
+        ASSERT_FALSE(string_is_safe(NULL, STRING_ALLOW_EMPTY));
+        ASSERT_FALSE(string_is_safe(NULL, STRING_ASCII));
+        ASSERT_FALSE(string_is_safe(NULL, STRING_ALLOW_BACKSLASHES));
+        ASSERT_FALSE(string_is_safe(NULL, STRING_ALLOW_QUOTES));
+        ASSERT_FALSE(string_is_safe(NULL, STRING_ALLOW_GLOBS));
+        ASSERT_FALSE(string_is_safe(NULL, STRING_FILENAME));
+
+        /* Baseline (flags=0): rejects empty, backslashes, quotes, globs, control chars and invalid UTF-8.
+         * Plain alphanumerics/whitespace and valid UTF-8 accepted. */
+        ASSERT_TRUE(string_is_safe("hello", 0));
+        ASSERT_TRUE(string_is_safe("hello world", 0));
+        ASSERT_TRUE(string_is_safe("über", 0));             /* valid UTF-8 allowed */
+        ASSERT_TRUE(string_is_safe("ünïcödé", 0));
+
+        ASSERT_FALSE(string_is_safe("", 0));                /* empty rejected by default */
+        ASSERT_FALSE(string_is_safe("a\\b", 0));            /* backslash rejected by default */
+        ASSERT_FALSE(string_is_safe("\"", 0));              /* double quote rejected by default */
+        ASSERT_FALSE(string_is_safe("'", 0));               /* single quote rejected by default */
+        ASSERT_FALSE(string_is_safe("*", 0));               /* glob rejected by default */
+        ASSERT_FALSE(string_is_safe("?", 0));               /* glob rejected by default */
+        ASSERT_FALSE(string_is_safe("[", 0));               /* glob rejected by default */
+        ASSERT_FALSE(string_is_safe("abc\x01", 0));         /* control char */
+        ASSERT_FALSE(string_is_safe("\t", 0));
+        ASSERT_FALSE(string_is_safe("\n", 0));
+        ASSERT_FALSE(string_is_safe("abc\x1f", 0));
+        ASSERT_FALSE(string_is_safe("abc\x7f", 0));         /* DEL */
+        ASSERT_FALSE(string_is_safe("ab\xc3\x28", 0));      /* invalid UTF-8 continuation */
+        ASSERT_FALSE(string_is_safe("\xff", 0));            /* not valid UTF-8 */
+
+        /* STRING_ALLOW_EMPTY. */
+        ASSERT_TRUE(string_is_safe("", STRING_ALLOW_EMPTY));
+        ASSERT_TRUE(string_is_safe("x", STRING_ALLOW_EMPTY));
+        ASSERT_TRUE(string_is_safe("hello", STRING_ALLOW_EMPTY));
+        ASSERT_FALSE(string_is_safe(NULL, STRING_ALLOW_EMPTY));
+
+        /* STRING_ASCII: high bytes rejected, low ASCII accepted, control chars still rejected.
+         * Empty is still rejected by default; backslashes/quotes/globs still rejected by default. */
+        ASSERT_TRUE(string_is_safe("hello", STRING_ASCII));
+        ASSERT_TRUE(string_is_safe("hello world 123!@#$%^&()", STRING_ASCII));
+        ASSERT_FALSE(string_is_safe("", STRING_ASCII));
+        ASSERT_FALSE(string_is_safe("über", STRING_ASCII));
+        ASSERT_FALSE(string_is_safe("\x80", STRING_ASCII));
+        ASSERT_FALSE(string_is_safe("\xff", STRING_ASCII));
+        ASSERT_FALSE(string_is_safe("abc\x01", STRING_ASCII));
+        ASSERT_FALSE(string_is_safe("abc\x7f", STRING_ASCII));
+        ASSERT_FALSE(string_is_safe("a\\b", STRING_ASCII));
+        ASSERT_FALSE(string_is_safe("a\"b", STRING_ASCII));
+        ASSERT_FALSE(string_is_safe("a*b", STRING_ASCII));
+
+        /* STRING_ALLOW_BACKSLASHES: backslashes allowed, quotes/globs still rejected. */
+        ASSERT_TRUE(string_is_safe("hello", STRING_ALLOW_BACKSLASHES));
+        ASSERT_TRUE(string_is_safe("hello world", STRING_ALLOW_BACKSLASHES));
+        ASSERT_TRUE(string_is_safe("\\", STRING_ALLOW_BACKSLASHES));
+        ASSERT_TRUE(string_is_safe("a\\b", STRING_ALLOW_BACKSLASHES));
+        ASSERT_TRUE(string_is_safe("foo\\", STRING_ALLOW_BACKSLASHES));
+        ASSERT_TRUE(string_is_safe("\\foo", STRING_ALLOW_BACKSLASHES));
+        ASSERT_TRUE(string_is_safe("foo\\nbar", STRING_ALLOW_BACKSLASHES)); /* literal backslash, not newline */
+        ASSERT_FALSE(string_is_safe("\"", STRING_ALLOW_BACKSLASHES));       /* quotes still rejected */
+        ASSERT_FALSE(string_is_safe("*", STRING_ALLOW_BACKSLASHES));        /* globs still rejected */
+
+        /* STRING_ALLOW_QUOTES: quotes allowed, backslashes/globs still rejected. */
+        ASSERT_TRUE(string_is_safe("hello", STRING_ALLOW_QUOTES));
+        ASSERT_TRUE(string_is_safe("hello world", STRING_ALLOW_QUOTES));
+        ASSERT_TRUE(string_is_safe("\"", STRING_ALLOW_QUOTES));
+        ASSERT_TRUE(string_is_safe("'", STRING_ALLOW_QUOTES));
+        ASSERT_TRUE(string_is_safe("hello\"world", STRING_ALLOW_QUOTES));
+        ASSERT_TRUE(string_is_safe("it's", STRING_ALLOW_QUOTES));
+        ASSERT_FALSE(string_is_safe("a\\b", STRING_ALLOW_QUOTES));          /* backslashes still rejected */
+        ASSERT_FALSE(string_is_safe("*", STRING_ALLOW_QUOTES));             /* globs still rejected */
+
+        /* STRING_ALLOW_GLOBS: globs allowed, backslashes/quotes still rejected. */
+        ASSERT_TRUE(string_is_safe("hello", STRING_ALLOW_GLOBS));
+        ASSERT_TRUE(string_is_safe("ab]c", STRING_ALLOW_GLOBS));            /* ']' is not in GLOB_CHARS anyway */
+        ASSERT_TRUE(string_is_safe("*", STRING_ALLOW_GLOBS));
+        ASSERT_TRUE(string_is_safe("?", STRING_ALLOW_GLOBS));
+        ASSERT_TRUE(string_is_safe("[", STRING_ALLOW_GLOBS));
+        ASSERT_TRUE(string_is_safe("foo*bar", STRING_ALLOW_GLOBS));
+        ASSERT_TRUE(string_is_safe("foo?bar", STRING_ALLOW_GLOBS));
+        ASSERT_TRUE(string_is_safe("foo[bar", STRING_ALLOW_GLOBS));
+        ASSERT_FALSE(string_is_safe("\"", STRING_ALLOW_GLOBS));             /* quotes still rejected */
+        ASSERT_FALSE(string_is_safe("a\\b", STRING_ALLOW_GLOBS));           /* backslashes still rejected */
+
+        /* STRING_FILENAME: rejects empty, ".", "..", and strings with '/'. */
+        ASSERT_TRUE(string_is_safe("hello", STRING_FILENAME));
+        ASSERT_TRUE(string_is_safe("hello.txt", STRING_FILENAME));
+        ASSERT_TRUE(string_is_safe("...", STRING_FILENAME));
+        ASSERT_TRUE(string_is_safe(".hidden", STRING_FILENAME));
+        ASSERT_FALSE(string_is_safe("", STRING_FILENAME));
+        ASSERT_FALSE(string_is_safe(".", STRING_FILENAME));
+        ASSERT_FALSE(string_is_safe("..", STRING_FILENAME));
+        ASSERT_FALSE(string_is_safe("/", STRING_FILENAME));
+        ASSERT_FALSE(string_is_safe("/foo", STRING_FILENAME));
+        ASSERT_FALSE(string_is_safe("foo/bar", STRING_FILENAME));
+
+        /* Pairwise combinations. */
+        ASSERT_TRUE(string_is_safe("", STRING_ALLOW_EMPTY | STRING_ASCII));
+        ASSERT_FALSE(string_is_safe("über", STRING_ALLOW_EMPTY | STRING_ASCII));
+        ASSERT_TRUE(string_is_safe("hello", STRING_ALLOW_EMPTY | STRING_ASCII));
+
+        ASSERT_TRUE(string_is_safe("ab\"cd", STRING_ALLOW_QUOTES | STRING_ALLOW_GLOBS));
+        ASSERT_TRUE(string_is_safe("ab*cd", STRING_ALLOW_QUOTES | STRING_ALLOW_GLOBS));
+        ASSERT_TRUE(string_is_safe("ab'*cd", STRING_ALLOW_QUOTES | STRING_ALLOW_GLOBS));
+        ASSERT_FALSE(string_is_safe("ab\\cd", STRING_ALLOW_QUOTES | STRING_ALLOW_GLOBS)); /* backslash still rejected */
+
+        ASSERT_TRUE(string_is_safe("hello.txt", STRING_FILENAME));
+        ASSERT_FALSE(string_is_safe("", STRING_FILENAME));
+        ASSERT_FALSE(string_is_safe("foo/bar", STRING_FILENAME));
+
+        ASSERT_TRUE(string_is_safe("foo?bar", STRING_ASCII | STRING_ALLOW_GLOBS));
+        ASSERT_FALSE(string_is_safe("foo\"bar", STRING_ASCII | STRING_ALLOW_GLOBS));      /* quotes still rejected */
+        ASSERT_FALSE(string_is_safe("über", STRING_ASCII | STRING_ALLOW_GLOBS));
+
+        ASSERT_TRUE(string_is_safe("foo\\bar", STRING_ALLOW_BACKSLASHES));
+        ASSERT_FALSE(string_is_safe("foo\"bar", STRING_ALLOW_BACKSLASHES));               /* quotes still rejected */
+        ASSERT_FALSE(string_is_safe("foo*bar", STRING_ALLOW_BACKSLASHES));                /* globs still rejected */
+        ASSERT_TRUE(string_is_safe("foo\\\"bar", STRING_ALLOW_BACKSLASHES | STRING_ALLOW_QUOTES));
+        ASSERT_TRUE(string_is_safe("foo\\bar", STRING_ALLOW_BACKSLASHES | STRING_ALLOW_QUOTES));
+        ASSERT_TRUE(string_is_safe("foo\"bar", STRING_ALLOW_BACKSLASHES | STRING_ALLOW_QUOTES));
+
+        /* All allow flags combined: only baseline (control chars, invalid UTF-8) and STRING_FILENAME apply. */
+        StringSafeFlags all = STRING_ALLOW_EMPTY | STRING_ASCII | STRING_ALLOW_BACKSLASHES | STRING_ALLOW_QUOTES | STRING_ALLOW_GLOBS | STRING_FILENAME;
+        ASSERT_TRUE(string_is_safe("hello.txt", all));
+        ASSERT_TRUE(string_is_safe("foo-bar_baz.conf", all));
+        ASSERT_TRUE(string_is_safe("a", all));
+        ASSERT_TRUE(string_is_safe("foo\\bar", all));            /* backslash allowed */
+        ASSERT_TRUE(string_is_safe("foo\"bar", all));            /* quote allowed */
+        ASSERT_TRUE(string_is_safe("foo'bar", all));             /* quote allowed */
+        ASSERT_TRUE(string_is_safe("foo*bar", all));             /* glob allowed */
+        ASSERT_TRUE(string_is_safe("foo?bar", all));             /* glob allowed */
+        ASSERT_TRUE(string_is_safe("foo[bar", all));             /* glob allowed */
+        ASSERT_FALSE(string_is_safe("", all));                   /* fails STRING_FILENAME */
+        ASSERT_FALSE(string_is_safe("über", all));               /* fails STRING_ASCII */
+        ASSERT_FALSE(string_is_safe("foo/bar", all));            /* fails STRING_FILENAME */
+        ASSERT_FALSE(string_is_safe(".", all));                  /* fails STRING_FILENAME */
+        ASSERT_FALSE(string_is_safe("..", all));                 /* fails STRING_FILENAME */
+        ASSERT_FALSE(string_is_safe("foo\x01""bar", all));       /* fails baseline control-char check */
+        ASSERT_FALSE(string_is_safe(NULL, all));
+}
+
 DEFINE_TEST_MAIN(LOG_DEBUG);
index 4a244ae83dc42835aba64a60cdac82e7ab195430..2e44aab963357ce75811673b74cc991b0738078e 100644 (file)
@@ -221,9 +221,6 @@ static int parse_options(const char *options) {
                         arg_hash_offset = off;
                 } else if ((val = startswith(word, "salt="))) {
 
-                        if (!string_is_safe(val))
-                                return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "salt= is not valid.");
-
                         if (isempty(val)) {
                                 arg_salt = mfree(arg_salt);
                                 arg_salt_size = 32;
index 5c70809e47c95801dcec1bfb477d41e0682f9b9e..5c0847b481472d29a45d97c696a6cffb08fa1ffc 100644 (file)
@@ -851,7 +851,12 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 OPTION_LONG("ssh-key-type", "TYPE", "Choose what type of SSH key to pass"):
-                        if (!string_is_safe(arg))
+                        if (isempty(arg)) {
+                                arg_ssh_key_type = mfree(arg_ssh_key_type);
+                                break;
+                        }
+
+                        if (!string_is_safe(arg, STRING_ALLOW_GLOBS))
                                 return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid value for --ssh-key-type=: %s", arg);
 
                         r = free_and_strdup_warn(&arg_ssh_key_type, arg);