diff --git a/src/analyze/analyze-compare-versions.c b/src/analyze/analyze-compare-versions.c index 5c15fd044d..a3293d73b7 100644 --- a/src/analyze/analyze-compare-versions.c +++ b/src/analyze/analyze-compare-versions.c @@ -17,9 +17,9 @@ int verb_compare_versions(int argc, char *argv[], uintptr_t _data, void *userdat /* We only output a warning on invalid version strings (instead of failing), since the comparison * functions try to handle invalid strings gracefully and it's still interesting to see what the * comparison result will be. */ - if (!version_is_valid_versionspec(v1)) + if (!version_is_valid(v1, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS|VERSION_ALLOW_EMPTY)) log_warning("Version string 1 contains disallowed characters, they will be treated as separators: %s", v1); - if (!version_is_valid_versionspec(v2)) + if (!version_is_valid(v2, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS|VERSION_ALLOW_EMPTY)) log_warning("Version string 2 contains disallowed characters, they will be treated as separators: %s", v2); if (argc == 3) { diff --git a/src/basic/string-util.c b/src/basic/string-util.c index a806d2ddfa..8de801a74b 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -1462,29 +1462,47 @@ char* find_line_after_internal(const char *haystack, const char *needle) { return NULL; } -bool version_is_valid(const char *s) { - if (isempty(s)) +bool version_is_valid(const char *s, VersionFlags flags) { + + /* Validates a version string superficially. This does not proces the version string in any + * semantical way, it mostly just validates that its charset is reasonable. */ + + if (FLAGS_SET(flags, VERSION_ALLOW_EMPTY) ? !s : isempty(s)) return false; if (!filename_part_is_valid(s)) return false; - /* This is a superset of the characters used by semver. We additionally allow "_". */ - if (!in_charset(s, ALPHANUMERICAL ".-+_")) - return false; + /* We always allow all characters specified by the UAPI.10 Version Specification, i.e. 0-9, a-z, A-Z, + * ".", "-", "~", "^". + * + * If the relevant flags are set we'll also allow "+" and "_" separators. + * + * Note that with SemVer allows 0-9, a-z, A-Z, "+", "-", ".", hence with VERSION_ALLOW_PLUS we + * implement a superset of it. + * + * If you wonder when to use which flags: when validating foreign versions (e.g. distribution + * versions in /etc/os-release or so) validate liberally, i.e. add + * VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS. When validating our own versioned objects (e.g. vpick + * or so) validate more strictly, and in particular refuse characters such as "_" and "+" that may be + * used for separating component names or boot attempt counters. Also: first – if appropriate – split + * the string into individual components. For example, if the string consists of a name and a + * version, separated by some character, only pass the version part to this function. The name part + * may pass verification, but it's cleaner to not rely on that. + * + * For details about UAPI.10 see: + * + * → https://uapi-group.org/specifications/specs/version_format_specification/ */ - return true; -} + char charset[] = ALPHANUMERICAL ".-~^" /* plus room for the two chars below: */ "\0\0"; + size_t l = strlen(charset); -bool version_is_valid_versionspec(const char *s) { + if (FLAGS_SET(flags, VERSION_ALLOW_UNDERSCORE)) + charset[l++] = '_'; + if (FLAGS_SET(flags, VERSION_ALLOW_PLUS)) + charset[l++] = '+'; - if (!filename_part_is_valid(s)) - return false; - - if (!in_charset(s, ALPHANUMERICAL ".-~^")) - return false; - - return true; + return in_charset(s, charset); } ssize_t strlevenshtein(const char *x, const char *y) { diff --git a/src/basic/string-util.h b/src/basic/string-util.h index 8f4cc69a54..d0b614f775 100644 --- a/src/basic/string-util.h +++ b/src/basic/string-util.h @@ -315,8 +315,13 @@ char* find_line_after_internal(const char *haystack, const char *needle); #define find_line_after(haystack, needle) \ const_generic(haystack, find_line_after_internal(haystack, needle)) -bool version_is_valid(const char *s) _pure_; -bool version_is_valid_versionspec(const char *s) _pure_; +typedef enum VersionFlags { + VERSION_ALLOW_EMPTY = 1 << 0, + VERSION_ALLOW_UNDERSCORE = 1 << 1, /* Allow "_" as separator (recommended separator) */ + VERSION_ALLOW_PLUS = 1 << 2, /* Allow "+" as separator (sometimes used as separator for boot attempt counters) */ +} VersionFlags; + +bool version_is_valid(const char *s, VersionFlags flags) _pure_; ssize_t strlevenshtein(const char *x, const char *y); diff --git a/src/bootctl/bootctl-link.c b/src/bootctl/bootctl-link.c index b470667085..ed1753af99 100644 --- a/src/bootctl/bootctl-link.c +++ b/src/bootctl/bootctl-link.c @@ -1492,7 +1492,7 @@ static int vl_link_prepare(sd_varlink *link, LinkParameters *p) { if (p->context.entry_title && !efi_loader_entry_title_valid(p->context.entry_title)) return sd_varlink_error_invalid_parameter_name(link, "entryTitle"); - if (p->context.entry_version && !version_is_valid_versionspec(p->context.entry_version)) + if (p->context.entry_version && !version_is_valid(p->context.entry_version, /* flags= */ 0)) return sd_varlink_error_invalid_parameter_name(link, "entryVersion"); if (p->context.entry_commit != 0 && !entry_commit_valid(p->context.entry_commit)) diff --git a/src/bootctl/bootctl.c b/src/bootctl/bootctl.c index 72d96bc1b0..238102095e 100644 --- a/src/bootctl/bootctl.c +++ b/src/bootctl/bootctl.c @@ -695,7 +695,7 @@ static int parse_argv(int argc, char *argv[], char ***ret_args) { break; } - if (!version_is_valid_versionspec(opts.arg)) + if (!version_is_valid(opts.arg, /* flags= */ 0)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Not a valid boot menu entry version: %s", opts.arg); r = free_and_strdup_warn(&arg_entry_version, opts.arg); diff --git a/src/libsystemd/sd-json/json-util.c b/src/libsystemd/sd-json/json-util.c index d69e37a3ce..6cbb61cb5e 100644 --- a/src/libsystemd/sd-json/json-util.c +++ b/src/libsystemd/sd-json/json-util.c @@ -449,7 +449,7 @@ int json_dispatch_const_version(const char *name, sd_json_variant *variant, sd_j return json_log(variant, flags, SYNTHETIC_ERRNO(EINVAL), "JSON field '%s' is not a string.", strna(name)); const char *version = sd_json_variant_string(variant); - if (!version_is_valid(version)) + if (!version_is_valid(version, FLAGS_SET(flags, SD_JSON_STRICT) ? 0 : VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)) return json_log(variant, flags, SYNTHETIC_ERRNO(EINVAL), "JSON field '%s' is not a valid version string.", strna(name)); *n = version; diff --git a/src/shared/vpick.c b/src/shared/vpick.c index c661f92faf..4f709c9e19 100644 --- a/src/shared/vpick.c +++ b/src/shared/vpick.c @@ -447,7 +447,7 @@ static int make_choice( *underscore = 0; } - if (!version_is_valid(e)) { + if (!version_is_valid(e, /* flags= */ 0)) { log_debug("Version string '%s' of entry '%s' is invalid, ignoring entry.", e, (*entry)->d_name); continue; } diff --git a/src/sysupdate/sysupdate-pattern.c b/src/sysupdate/sysupdate-pattern.c index 459c8fda29..294499f4af 100644 --- a/src/sysupdate/sysupdate-pattern.c +++ b/src/sysupdate/sysupdate-pattern.c @@ -268,7 +268,7 @@ int pattern_match(const char *pattern, const char *s, InstanceMetadata *ret) { switch (e->type) { case PATTERN_VERSION: - if (!version_is_valid(t)) { + if (!version_is_valid(t, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)) { log_debug("Version string is not valid, refusing: %s", t); goto nope; } diff --git a/src/sysupdate/sysupdate-transfer.c b/src/sysupdate/sysupdate-transfer.c index 30e879574f..e80dcaf39a 100644 --- a/src/sysupdate/sysupdate-transfer.c +++ b/src/sysupdate/sysupdate-transfer.c @@ -141,7 +141,7 @@ static int config_parse_protect_version( return 0; } - if (!version_is_valid(resolved)) { + if (!version_is_valid(resolved, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)) { log_syntax(unit, LOG_WARNING, filename, line, 0, "ProtectVersion= string is not valid, ignoring: %s", resolved); return 0; @@ -180,7 +180,7 @@ static int config_parse_min_version( return 0; } - if (!version_is_valid(rvalue)) { + if (!version_is_valid(resolved, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)) { log_syntax(unit, LOG_WARNING, filename, line, 0, "MinVersion= string is not valid, ignoring: %s", resolved); return 0; diff --git a/src/sysupdate/sysupdated.c b/src/sysupdate/sysupdated.c index bc5a2439b5..9bbcc870bc 100644 --- a/src/sysupdate/sysupdated.c +++ b/src/sysupdate/sysupdated.c @@ -952,7 +952,7 @@ static int target_method_describe(sd_bus_message *msg, void *userdata, sd_bus_er if (r < 0) return r; - if (!version_is_valid(version)) + if (!version_is_valid(version, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid version"); if ((flags & ~SD_SYSUPDATE_FLAGS_ALL) != 0) @@ -1102,7 +1102,7 @@ static int target_method_acquire(sd_bus_message *msg, void *userdata, sd_bus_err if (isempty(version)) action = "org.freedesktop.sysupdate1.update"; else { - if (!version_is_valid(version)) + if (!version_is_valid(version, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid version"); action = "org.freedesktop.sysupdate1.update-to-version"; @@ -1190,7 +1190,7 @@ static int target_method_install(sd_bus_message *msg, void *userdata, sd_bus_err if (isempty(version)) action = "org.freedesktop.sysupdate1.update"; else { - if (!version_is_valid(version)) + if (!version_is_valid(version, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid version"); action = "org.freedesktop.sysupdate1.update-to-version"; diff --git a/src/test/test-string-util.c b/src/test/test-string-util.c index 970421c7ab..a18999cf8a 100644 --- a/src/test/test-string-util.c +++ b/src/test/test-string-util.c @@ -1408,13 +1408,13 @@ TEST(strstrafter) { } TEST(version_is_valid) { - assert_se(!version_is_valid(NULL)); - assert_se(!version_is_valid("")); - assert_se(version_is_valid("0")); - assert_se(version_is_valid("5")); - assert_se(version_is_valid("999999")); - assert_se(version_is_valid("999999.5")); - assert_se(version_is_valid("6.2.12-300.fc38.x86_64")); + assert_se(!version_is_valid(NULL, /* flags= */ 0)); + assert_se(!version_is_valid("", /* flags= */ 0)); + assert_se(version_is_valid("0", /* flags= */ 0)); + assert_se(version_is_valid("5", /* flags= */ 0)); + assert_se(version_is_valid("999999", /* flags= */ 0)); + assert_se(version_is_valid("999999.5", /* flags= */ 0)); + assert_se(version_is_valid("6.2.12-300.fc38.x86_64", /* flags= */ VERSION_ALLOW_UNDERSCORE)); } TEST(strextendn) { diff --git a/src/vpick/vpick-tool.c b/src/vpick/vpick-tool.c index 85f2b5c1cb..8dd22e0ea2 100644 --- a/src/vpick/vpick-tool.c +++ b/src/vpick/vpick-tool.c @@ -115,7 +115,7 @@ static int parse_argv(int argc, char *argv[], char ***ret_args) { break; OPTION_SHORT('V', "VERSION", "Look for specified version"): - if (!version_is_valid(opts.arg)) + if (!version_is_valid(opts.arg, /* flags= */ 0)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid version string: %s", opts.arg); r = free_and_strdup_warn(&arg_filter_version, opts.arg);