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 3a005df08e5206217e25affa8f5e94d21871e024..16e95463613a25f4c3d006f84f47bc70aa4392a2 100644 (file)
--- 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 480a3f33eb2ef8596ff800fdd64856b681f949c3..70567daba238f18af0642f0c33043ea9f58b111d 100644 (file)
--- 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;