From ac3a2d74d870924fc00dcf7edd3f8ca63941dad7 Mon Sep 17 00:00:00 2001 From: Iker Pedrosa Date: Mon, 15 Dec 2025 12:42:46 +0100 Subject: [PATCH 1/5] tests/system/tests/test_groupmod.py: add test for groupmod -U with empty string Test verifies that groupmod -U '' correctly clears group membership. Signed-off-by: Iker Pedrosa --- tests/system/tests/test_groupmod.py | 31 +++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/system/tests/test_groupmod.py b/tests/system/tests/test_groupmod.py index 999ddc0cbf..6617602f5b 100644 --- a/tests/system/tests/test_groupmod.py +++ b/tests/system/tests/test_groupmod.py @@ -38,3 +38,34 @@ def test_groupmod__change_gid(shadow: Shadow): assert gshadow_entry is not None, "Group should be found" assert gshadow_entry.name == "tgroup", "Incorrect groupname" assert gshadow_entry.password == "!", "Incorrect password" + + +@pytest.mark.topology(KnownTopology.Shadow) +def test_groupmod__u_option_empty_string_clears_members(shadow: Shadow): + """ + :title: Test groupmod -U option with empty user list + :setup: + 1. Create test group + :steps: + 1. Run groupmod with -U option and empty string parameter + 2. Verify group exists after operation + 3. Confirm group has no members + :expectedresults: + 1. groupmod -U '' command completes successfully + 2. Group entry remains valid and accessible + 3. Group member list is empty (no users assigned to group) + :customerscenario: False + """ + shadow.groupadd("tgroup") + shadow.groupmod("-U '' tgroup") + + group_entry = shadow.tools.getent.group("tgroup") + assert group_entry is not None, "Group should be found" + assert group_entry.name == "tgroup", "Incorrect groupname" + assert not group_entry.members, "Group should have no members" + + if shadow.host.features["gshadow"]: + gshadow_entry = shadow.tools.getent.gshadow("tgroup") + assert gshadow_entry is not None, "Group should be found" + assert gshadow_entry.name == "tgroup", "Incorrect groupname" + assert not gshadow_entry.members, "Group should have no members" From 44bba1dcd223a534197e72f46b0b5ad99a660d9d Mon Sep 17 00:00:00 2001 From: Iker Pedrosa Date: Mon, 15 Dec 2025 12:47:08 +0100 Subject: [PATCH 2/5] tests/system/tests/test_groupadd.py: add test for groupadd -U with empty string Test verifies that groupadd -U '' correctly creates group with no members. Signed-off-by: Iker Pedrosa --- tests/system/tests/test_groupadd.py | 30 +++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/system/tests/test_groupadd.py b/tests/system/tests/test_groupadd.py index a979919de4..b46621b6ec 100644 --- a/tests/system/tests/test_groupadd.py +++ b/tests/system/tests/test_groupadd.py @@ -36,3 +36,33 @@ def test_groupadd__add_group(shadow: Shadow): assert gshadow_entry is not None, "Group should be found" assert gshadow_entry.name == "tgroup", "Incorrect groupname" assert gshadow_entry.password == "!", "Incorrect password" + + +@pytest.mark.topology(KnownTopology.Shadow) +def test_groupadd__u_option_empty_string_clears_members(shadow: Shadow): + """ + :title: Test groupadd -U option with empty user list + :setup: + 1. None required + :steps: + 1. Run groupadd with -U option and empty string parameter + 2. Verify group exists after creation + 3. Confirm group has no members + :expectedresults: + 1. groupadd -U '' command completes successfully + 2. Group entry is created and accessible + 3. Group member list is empty (no users assigned to group) + :customerscenario: False + """ + shadow.groupadd("-U '' tgroup") + + group_entry = shadow.tools.getent.group("tgroup") + assert group_entry is not None, "Group should be found" + assert group_entry.name == "tgroup", "Incorrect groupname" + assert not group_entry.members, "Group should have no members" + + if shadow.host.features["gshadow"]: + gshadow_entry = shadow.tools.getent.gshadow("tgroup") + assert gshadow_entry is not None, "Group should be found" + assert gshadow_entry.name == "tgroup", "Incorrect groupname" + assert not gshadow_entry.members, "Group should have no members" From 2e1296d63ec1084b181dbdbeb2c216d45a3e3175 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Mon, 22 Dec 2025 20:05:36 +0100 Subject: [PATCH 3/5] lib/, src/: Add blocks This is in preparation for the following patch. Signed-off-by: Alejandro Colomar --- lib/addgrps.c | 18 ++++++++++-------- src/groupmod.c | 20 +++++++++++--------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/lib/addgrps.c b/lib/addgrps.c index 6ab34dcb31..da7843f231 100644 --- a/lib/addgrps.c +++ b/lib/addgrps.c @@ -52,16 +52,18 @@ add_groups(const char *list) if (dup == NULL) goto free_gids; - while (NULL != (g = strsep(&p, ",:"))) { - struct group *grp; + { + while (NULL != (g = strsep(&p, ",:"))) { + struct group *grp; - grp = getgrnam(g); /* local, no need for xgetgrnam */ - if (NULL == grp) { - fprintf(shadow_logfd, _("Warning: unknown group %s\n"), g); - continue; - } + grp = getgrnam(g); /* local, no need for xgetgrnam */ + if (NULL == grp) { + fprintf(shadow_logfd, _("Warning: unknown group %s\n"), g); + continue; + } - LSEARCH(gid_t, &grp->gr_gid, gids, &n); + LSEARCH(gid_t, &grp->gr_gid, gids, &n); + } } free(dup); diff --git a/src/groupmod.c b/src/groupmod.c index 22d07fe69b..4f9c9ea5fd 100644 --- a/src/groupmod.c +++ b/src/groupmod.c @@ -282,18 +282,20 @@ grp_update(void) } #endif /* SHADOWGRP */ - ul = user_list; - while (NULL != (u = strsep(&ul, ","))) { - if (prefix_getpwnam(u) == NULL) { - fprintf(stderr, _("Invalid member username %s\n"), u); - exit (E_GRP_UPDATE); - } + { + ul = user_list; + while (NULL != (u = strsep(&ul, ","))) { + if (prefix_getpwnam(u) == NULL) { + fprintf(stderr, _("Invalid member username %s\n"), u); + exit(E_GRP_UPDATE); + } - grp.gr_mem = add_list(grp.gr_mem, u); + grp.gr_mem = add_list(grp.gr_mem, u); #ifdef SHADOWGRP - if (NULL != osgrp) - sgrp.sg_mem = add_list(sgrp.sg_mem, u); + if (NULL != osgrp) + sgrp.sg_mem = add_list(sgrp.sg_mem, u); #endif /* SHADOWGRP */ + } } } From 0f31bb63fbfd29d5ebc7b1a89aaa4429bcdff705 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Sun, 14 Dec 2025 00:51:34 +0100 Subject: [PATCH 4/5] lib/, src/: Some empty lists have 0 elements, not 1 empty string In general, empty fields in a CSV are errors. However, in some cases, we want to allow passing empty lists, and the way to encode that is as an empty string. This was accidentally broken in 4.17.0, when we switched from using strtok(3) to strsep(3), without remembering to special-case an empty CSV. The bug affected directly groupadd(8) and groupmod(8). The bug also affected the library function add_groups(). In systems using PAM, that function is unused. On systems without PAM, it is called by the library function setup_uid_gid(), with the contents of the "CONSOLE_GROUPS" configuration (login.defs) CSV string. setup_uid_gid() is directly called by su(1) and login(1) on systems without PAM. setup_uid_gid() is also called by the library function expire(). expire() is directly called by expiry(1), su(1), and login(1). This bug is a regression introduced in the release 4.17.0, and present in the releases 4.17.{0..4} and 4.18.0. Fixes: 90afe61003ef (2024-12-05; "lib/, src/: Use strsep(3) instead of strtok(3)") Link: Reported-by: Osark Vieira Signed-off-by: Alejandro Colomar --- lib/addgrps.c | 3 ++- src/groupadd.c | 3 ++- src/groupmod.c | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/addgrps.c b/lib/addgrps.c index da7843f231..7768f1325f 100644 --- a/lib/addgrps.c +++ b/lib/addgrps.c @@ -24,6 +24,7 @@ #include "shadow/grp/agetgroups.h" #include "shadowlog.h" #include "string/strchr/strchrscnt.h" +#include "string/strcmp/streq.h" #include "string/strerrno.h" @@ -52,7 +53,7 @@ add_groups(const char *list) if (dup == NULL) goto free_gids; - { + if (!streq(dup, "")) { while (NULL != (g = strsep(&p, ",:"))) { struct group *grp; diff --git a/src/groupadd.c b/src/groupadd.c index 7bb946b3c1..fab8111b4a 100644 --- a/src/groupadd.c +++ b/src/groupadd.c @@ -40,6 +40,7 @@ #include "shadow/gshadow/sgrp.h" #include "shadowlog.h" #include "string/memset/memzero.h" +#include "string/strcmp/streq.h" #include "string/strerrno.h" #include "string/strtok/stpsep.h" @@ -217,7 +218,7 @@ grp_update(void) } #endif /* SHADOWGRP */ - if (user_list) { + if (user_list && !streq(user_list, "")) { char *u, *ul; ul = user_list; diff --git a/src/groupmod.c b/src/groupmod.c index 4f9c9ea5fd..5b90c2342c 100644 --- a/src/groupmod.c +++ b/src/groupmod.c @@ -282,7 +282,7 @@ grp_update(void) } #endif /* SHADOWGRP */ - { + if (!streq(user_list, "")) { ul = user_list; while (NULL != (u = strsep(&ul, ","))) { if (prefix_getpwnam(u) == NULL) { From a893c004d1ad82d53d8721d1554f99630e26ac31 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Sun, 14 Dec 2025 00:57:19 +0100 Subject: [PATCH 5/5] lib/, src/: Reduce scope of local variables Signed-off-by: Alejandro Colomar --- lib/addgrps.c | 7 +++++-- src/groupmod.c | 4 ++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/addgrps.c b/lib/addgrps.c index 7768f1325f..b90828e1e2 100644 --- a/lib/addgrps.c +++ b/lib/addgrps.c @@ -36,7 +36,7 @@ int add_groups(const char *list) { - char *g, *p, *dup; + char *dup; FILE *shadow_logfd = log_get_logfd(); gid_t *gids; size_t n; @@ -49,11 +49,14 @@ add_groups(const char *list) if (gids == NULL) return -1; - p = dup = strdup(list); + dup = strdup(list); if (dup == NULL) goto free_gids; if (!streq(dup, "")) { + char *g, *p; + + p = dup; while (NULL != (g = strsep(&p, ",:"))) { struct group *grp; diff --git a/src/groupmod.c b/src/groupmod.c index 5b90c2342c..c29f905088 100644 --- a/src/groupmod.c +++ b/src/groupmod.c @@ -259,8 +259,6 @@ grp_update(void) } if (user_list) { - char *u, *ul; - if (!aflg) { // requested to replace the existing groups grp.gr_mem = xmalloc_T(1, char *); @@ -283,6 +281,8 @@ grp_update(void) #endif /* SHADOWGRP */ if (!streq(user_list, "")) { + char *u, *ul; + ul = user_list; while (NULL != (u = strsep(&ul, ","))) { if (prefix_getpwnam(u) == NULL) {