string-util: beef up string_is_safe()

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                              │
  ╰───────────────────────────────────────────────────┴──────────────────────────────────────────────╯
This commit is contained in:
Lennart Poettering
2026-04-21 17:07:53 +02:00
committed by Lennart Poettering
parent a84bbd15d9
commit 812aa57d2c
26 changed files with 238 additions and 85 deletions
+24 -9
View 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 (strchr(QUOTES "\\\x7f", *t))
if (!FLAGS_SET(flags, STRING_ALLOW_BACKSLASHES) && *t == '\\')
return false;
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) {
+10 -2
View 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) {
+2 -10
View 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;
}
+1 -2
View 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));
+5 -2
View 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);
+1 -1
View 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);
+1 -1
View 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;
}
+10 -6
View 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)
+1 -1
View 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)
+1 -1
View 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);
+1 -1
View 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);
+3 -1
View 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);
+4 -6
View 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);
+3 -3
View 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));
+1 -1
View 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);
}
+2 -2
View 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;
}
+2 -2
View 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)
+1 -2
View File
@@ -11,10 +11,9 @@
#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) {
+3 -3
View 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);
+1 -12
View 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;
}
+1 -2
View 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 */
}
+4 -4
View 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) {
+8 -7
View 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))
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 is not valid UTF-8: %s", optarg);
if (string_has_cc(optarg, NULL))
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:
+142
View 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);
-3
View 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;
+6 -1
View 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);