user-util: stop mangling groupname in get_group_creds

The param was input/output, which is unexpected and confusing and actually made most callers much more complicated than they needed to be. The function was playing fast and loose with the return value. In some cases it was returning a static string, which would be completely fine. But in other case it was returning a pointer into the getgrgid static buffer, i.e. that return value could be overwritten. AFAICT, this didn't matter in any of the callers, but we shouldn't do that anyway. So use a separate output param with an allocated string that the caller is responsible for. It turns out that all callers pass NULL (outside of tests) and zero for flags. So the function _could_ be simplified. But get_user_creds is called with all the params actually used, and I think having the the functions so different wouldn't be nice. I first wrote a commit to drop the unused params, but then I discarded it. The error message in exec-invoke.c is fixed.
2026-05-21 18:02:05 +02:00
parent 3d0a05dcc3
commit 3ecd6b69cf
8 changed files with 64 additions and 105 deletions
@@ -352,91 +352,75 @@ int get_user_creds(
        return 0;
 }

-static int synthesize_group_creds(
-                const char **groupname,
-                gid_t *ret_gid) {
-
+static int synthesize_group_creds(const char *groupname, char **ret_name, gid_t *ret_gid) {
        assert(groupname);
-        assert(*groupname);

-        if (STR_IN_SET(*groupname, "root", "0")) {
-                *groupname = "root";
+        gid_t id;
+        const char *n;
+        int r;

-                if (ret_gid)
-                        *ret_gid = 0;
+        if (STR_IN_SET(groupname, "root", "0")) {
+                id = 0;
+                n = "root";
+        } else if (STR_IN_SET(groupname, NOBODY_GROUP_NAME, "65534") &&
+                   synthesize_nobody()) {
+                id = GID_NOBODY;
+                n = NOBODY_GROUP_NAME;
+        } else
+                return -ENOMEDIUM;

-                return 0;
-        }
-
-        if (STR_IN_SET(*groupname, NOBODY_GROUP_NAME, "65534") &&
-            synthesize_nobody()) {
-                *groupname = NOBODY_GROUP_NAME;
-
-                if (ret_gid)
-                        *ret_gid = GID_NOBODY;
-
-                return 0;
-        }
-
-        return -ENOMEDIUM;
+        r = strdup_to_full(ret_name, n);
+        if (r < 0)
+                return r;
+        if (ret_gid)
+                *ret_gid = id;
+        return 0;
 }

-int get_group_creds(const char **groupname, gid_t *ret_gid, UserCredsFlags flags) {
-        bool patch_groupname = false;
-        struct group *g;
+int get_group_creds(const char *groupname, UserCredsFlags flags, char **ret_name, gid_t *ret_gid) {
+        _cleanup_free_ struct group *gr = NULL;
        gid_t id;
        int r;

        assert(groupname);
-        assert(*groupname);

        if (!FLAGS_SET(flags, USER_CREDS_PREFER_NSS)) {
-                r = synthesize_group_creds(groupname, ret_gid);
+                r = synthesize_group_creds(groupname, ret_name, ret_gid);
                if (r >= 0)
                        return 0;
                if (r != -ENOMEDIUM) /* not a groupname we can synthesize */
                        return r;
        }

-        if (parse_gid(*groupname, &id) >= 0) {
-                errno = 0;
-                g = getgrgid(id);
-
-                if (g)
-                        patch_groupname = true;
+        if (parse_gid(groupname, &id) >= 0) {
+                r = getgrgid_malloc(id, &gr);
+                if (r >= 0)
+                        groupname = gr->gr_name;
                else if (FLAGS_SET(flags, USER_CREDS_ALLOW_MISSING)) {
                        if (ret_gid)
                                *ret_gid = id;
-
+                        if (ret_name)
+                                *ret_name = NULL;
                        return 0;
                }
-        } else {
-                errno = 0;
-                g = getgrnam(*groupname);
-        }
-
-        if (!g) {
-                /* getgrnam() may fail with ENOENT if /etc/group is missing.
-                 * For us that is equivalent to the name not being defined. */
-                r = IN_SET(errno, 0, ENOENT) ? -ESRCH : -errno;
-
-                if (FLAGS_SET(flags, USER_CREDS_PREFER_NSS))
-                        if (synthesize_group_creds(groupname, ret_gid) >= 0)
-                                return 0;
+        } else
+                r = getgrnam_malloc(groupname, &gr);

+        if (r < 0) {
+                if (FLAGS_SET(flags, USER_CREDS_PREFER_NSS) &&
+                    synthesize_group_creds(groupname, ret_name, ret_gid) >= 0)
+                        return 0;
                return r;
        }

-        if (ret_gid) {
-                if (!gid_is_valid(g->gr_gid))
-                        return -EBADMSG;
-
-                *ret_gid = g->gr_gid;
-        }
-
-        if (patch_groupname)
-                *groupname = g->gr_name;
+        if (ret_gid && !gid_is_valid(gr->gr_gid))
+                return -EBADMSG;

+        r = strdup_to_full(ret_name, groupname);
+        if (r < 0)
+                return r;
+        if (ret_gid)
+                *ret_gid = gr->gr_gid;
        return 0;
 }

@@ -521,7 +505,7 @@ int in_group(const char *name) {
        int r;
        gid_t gid;

-        r = get_group_creds(&name, &gid, 0);
+        r = get_group_creds(name, /* flags= */ 0, /* ret_name= */ NULL, &gid);
        if (r < 0)
                return r;

@@ -63,7 +63,7 @@ typedef enum UserCredsFlags {
 } UserCredsFlags;

 int get_user_creds(const char **username, uid_t *ret_uid, gid_t *ret_gid, const char **ret_home, const char **ret_shell, UserCredsFlags flags);
-int get_group_creds(const char **groupname, gid_t *ret_gid, UserCredsFlags flags);
+int get_group_creds(const char *groupname, UserCredsFlags flags, char **ret_name, gid_t *ret_gid);

 char* uid_to_name(uid_t uid);
 char* gid_to_name(gid_t gid);