From: logical-misha <220645577+logical-misha@users.noreply.github.com> Date: Fri, 17 Jul 2026 23:17:02 +0000 (+0000) Subject: user-record: validate JSON shell fields with valid_shell() X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=841ebcbb7d6c5135d52dd180bc4bae8ce9a3d655;p=thirdparty%2Fsystemd.git user-record: validate JSON shell fields with valid_shell() The user record loader currently uses a generic filename-or-path check for shell and fallbackShell. This allows values that homectl rejects, including relative names, control characters, colons, and trailing slashes. Use valid_shell() for all three record locations and cover the top-level, matching per-machine, and status fallback fields at the loader boundary. Fixes #43066 --- diff --git a/src/shared/user-record.c b/src/shared/user-record.c index 3a005df08e5..16e95463613 100644 --- a/src/shared/user-record.c +++ b/src/shared/user-record.c @@ -394,7 +394,7 @@ static int json_dispatch_rlimits(const char *name, sd_json_variant *variant, sd_ return 0; } -static int json_dispatch_filename_or_path(const char *name, sd_json_variant *variant, sd_json_dispatch_flags_t flags, void *userdata) { +static int json_dispatch_shell(const char *name, sd_json_variant *variant, sd_json_dispatch_flags_t flags, void *userdata) { char **s = ASSERT_PTR(userdata); const char *n; int r; @@ -408,8 +408,8 @@ static int json_dispatch_filename_or_path(const char *name, sd_json_variant *var return json_log(variant, flags, SYNTHETIC_ERRNO(EINVAL), "JSON field '%s' is not a string.", strna(name)); n = sd_json_variant_string(variant); - if (!filename_is_valid(n) && !path_is_normalized(n)) - return json_log(variant, flags, SYNTHETIC_ERRNO(EINVAL), "JSON field '%s' is not a valid file name or normalized path.", strna(name)); + if (!valid_shell(n)) + return json_log(variant, flags, SYNTHETIC_ERRNO(EINVAL), "JSON field '%s' is not a valid shell path.", strna(name)); r = free_and_strdup(s, n); if (r < 0) @@ -1282,7 +1282,7 @@ static int dispatch_per_machine(const char *name, sd_json_variant *variant, sd_j { "blobManifest", SD_JSON_VARIANT_OBJECT, dispatch_blob_manifest, offsetof(UserRecord, blob_manifest), 0 }, { "iconName", SD_JSON_VARIANT_STRING, sd_json_dispatch_string, offsetof(UserRecord, icon_name), SD_JSON_STRICT }, { "location", SD_JSON_VARIANT_STRING, sd_json_dispatch_string, offsetof(UserRecord, location), 0 }, - { "shell", SD_JSON_VARIANT_STRING, json_dispatch_filename_or_path, offsetof(UserRecord, shell), 0 }, + { "shell", SD_JSON_VARIANT_STRING, json_dispatch_shell, offsetof(UserRecord, shell), 0 }, { "umask", _SD_JSON_VARIANT_TYPE_INVALID, json_dispatch_access_mode, offsetof(UserRecord, umask), SD_JSON_STRICT }, { "environment", SD_JSON_VARIANT_ARRAY, json_dispatch_strv_environment, offsetof(UserRecord, environment), 0 }, { "timeZone", SD_JSON_VARIANT_STRING, sd_json_dispatch_string, offsetof(UserRecord, time_zone), SD_JSON_STRICT }, @@ -1409,7 +1409,7 @@ static int dispatch_status(const char *name, sd_json_variant *variant, sd_json_d { "removable", SD_JSON_VARIANT_BOOLEAN, sd_json_dispatch_tristate, offsetof(UserRecord, removable), 0 }, { "accessMode", _SD_JSON_VARIANT_TYPE_INVALID, json_dispatch_access_mode, offsetof(UserRecord, access_mode), 0 }, { "fileSystemType", SD_JSON_VARIANT_STRING, sd_json_dispatch_string, offsetof(UserRecord, file_system_type), SD_JSON_STRICT }, - { "fallbackShell", SD_JSON_VARIANT_STRING, json_dispatch_filename_or_path, offsetof(UserRecord, fallback_shell), 0 }, + { "fallbackShell", SD_JSON_VARIANT_STRING, json_dispatch_shell, offsetof(UserRecord, fallback_shell), 0 }, { "fallbackHomeDirectory", SD_JSON_VARIANT_STRING, json_dispatch_home_directory, offsetof(UserRecord, fallback_home_directory), 0 }, { "useFallback", SD_JSON_VARIANT_BOOLEAN, sd_json_dispatch_stdbool, offsetof(UserRecord, use_fallback), 0 }, { "defaultArea", SD_JSON_VARIANT_STRING, json_dispatch_filename, offsetof(UserRecord, default_area), 0 }, @@ -1651,7 +1651,7 @@ int user_record_load(UserRecord *h, sd_json_variant *v, UserRecordLoadFlags load { "disposition", SD_JSON_VARIANT_STRING, json_dispatch_user_disposition, offsetof(UserRecord, disposition), 0 }, { "lastChangeUSec", _SD_JSON_VARIANT_TYPE_INVALID, sd_json_dispatch_uint64, offsetof(UserRecord, last_change_usec), 0 }, { "lastPasswordChangeUSec", _SD_JSON_VARIANT_TYPE_INVALID, sd_json_dispatch_uint64, offsetof(UserRecord, last_password_change_usec), 0 }, - { "shell", SD_JSON_VARIANT_STRING, json_dispatch_filename_or_path, offsetof(UserRecord, shell), 0 }, + { "shell", SD_JSON_VARIANT_STRING, json_dispatch_shell, offsetof(UserRecord, shell), 0 }, { "umask", _SD_JSON_VARIANT_TYPE_INVALID, json_dispatch_access_mode, offsetof(UserRecord, umask), SD_JSON_STRICT }, { "environment", SD_JSON_VARIANT_ARRAY, json_dispatch_strv_environment, offsetof(UserRecord, environment), 0 }, { "timeZone", SD_JSON_VARIANT_STRING, sd_json_dispatch_string, offsetof(UserRecord, time_zone), SD_JSON_STRICT }, diff --git a/src/test/test-user-record.c b/src/test/test-user-record.c index 480a3f33eb2..70567daba23 100644 --- a/src/test/test-user-record.c +++ b/src/test/test-user-record.c @@ -1,7 +1,9 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ +#include "sd-id128.h" #include "sd-json.h" +#include "id128-util.h" #include "tests.h" #include "user-record.h" @@ -13,6 +15,100 @@ 0; \ }) +TEST(shell_validation) { + static const char * const invalid_shells[] = { + "sh", + "/bin/sh\nbad", + "/bin/sh:bad", + "/bin/sh/", + }; + + _cleanup_(sd_json_variant_unrefp) sd_json_variant *v = NULL; + _cleanup_(user_record_unrefp) UserRecord *u = NULL; + sd_id128_t mid; + int r; + + FOREACH_ELEMENT(shell, invalid_shells) { + ASSERT_ERROR(user_record_build( + &u, + SD_JSON_BUILD_OBJECT( + SD_JSON_BUILD_PAIR_STRING("disposition", "regular"), + SD_JSON_BUILD_PAIR_STRING("userName", "test"), + SD_JSON_BUILD_PAIR_STRING("shell", *shell))), + EINVAL); + ASSERT_NULL(u); + } + + ASSERT_OK(sd_json_buildo(&v, + SD_JSON_BUILD_PAIR_STRING("disposition", "regular"), + SD_JSON_BUILD_PAIR_STRING("userName", "test"), + SD_JSON_BUILD_PAIR_STRING("shell", "bash"))); + ASSERT_NOT_NULL(u = user_record_new()); + ASSERT_OK(user_record_load(u, v, USER_RECORD_LOAD_FULL|USER_RECORD_PERMISSIVE)); + ASSERT_STREQ(u->user_name, "test"); + ASSERT_NULL(u->shell); + ASSERT_STREQ(user_record_shell(u), DEFAULT_USER_SHELL); + u = user_record_unref(u); + + USER(&u, + SD_JSON_BUILD_PAIR_STRING("userName", "test"), + SD_JSON_BUILD_PAIR_STRING("shell", "/bin/sh")); + ASSERT_STREQ(u->shell, "/bin/sh"); + ASSERT_STREQ(user_record_shell(u), "/bin/sh"); + u = user_record_unref(u); + + r = sd_id128_get_machine(&mid); + if (ERRNO_IS_NEG_MACHINE_ID_UNSET(r)) + return (void) log_tests_skipped("/etc/machine-id missing"); + ASSERT_OK(r); + + FOREACH_ELEMENT(shell, invalid_shells) { + ASSERT_ERROR(user_record_build( + &u, + SD_JSON_BUILD_OBJECT( + SD_JSON_BUILD_PAIR_STRING("disposition", "regular"), + SD_JSON_BUILD_PAIR_STRING("userName", "test"), + SD_JSON_BUILD_PAIR_ARRAY( + "perMachine", + SD_JSON_BUILD_OBJECT( + SD_JSON_BUILD_PAIR_ID128("matchMachineId", mid), + SD_JSON_BUILD_PAIR_STRING("shell", *shell))))), + EINVAL); + ASSERT_NULL(u); + + ASSERT_ERROR(user_record_build( + &u, + SD_JSON_BUILD_OBJECT( + SD_JSON_BUILD_PAIR_STRING("disposition", "regular"), + SD_JSON_BUILD_PAIR_STRING("userName", "test"), + SD_JSON_BUILD_PAIR_OBJECT( + "status", + SD_JSON_BUILD_PAIR_OBJECT( + SD_ID128_TO_STRING(mid), + SD_JSON_BUILD_PAIR_STRING("fallbackShell", *shell))))), + EINVAL); + ASSERT_NULL(u); + } + + USER(&u, + SD_JSON_BUILD_PAIR_STRING("userName", "test"), + SD_JSON_BUILD_PAIR_STRING("shell", "/bin/sh"), + SD_JSON_BUILD_PAIR_ARRAY( + "perMachine", + SD_JSON_BUILD_OBJECT( + SD_JSON_BUILD_PAIR_ID128("matchMachineId", mid), + SD_JSON_BUILD_PAIR_STRING("shell", "/bin/bash"))), + SD_JSON_BUILD_PAIR_OBJECT( + "status", + SD_JSON_BUILD_PAIR_OBJECT( + SD_ID128_TO_STRING(mid), + SD_JSON_BUILD_PAIR_STRING("fallbackShell", "/bin/zsh"), + SD_JSON_BUILD_PAIR_BOOLEAN("useFallback", true)))); + ASSERT_STREQ(u->shell, "/bin/bash"); + ASSERT_STREQ(u->fallback_shell, "/bin/zsh"); + ASSERT_STREQ(user_record_shell(u), "/bin/zsh"); +} + TEST(self_changes) { _cleanup_(user_record_unrefp) UserRecord *curr = NULL, *new = NULL;