From 750a1390eef0284aad3efe8bdbc640ec1d3d925b Mon Sep 17 00:00:00 2001 From: lixinyun Date: Mon, 26 May 2025 12:34:16 +0800 Subject: [PATCH] =?UTF-8?q?=E5=90=8C=E6=AD=A5=E4=BF=AE=E5=A4=8D=E7=A4=BE?= =?UTF-8?q?=E5=8C=BAgroupmod=20-U=E7=BC=BA=E9=99=B7?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: lixinyun --- backport-src-groupmod-fix-double-free.patch | 53 +++++++++++++++++++++ shadow.spec | 6 ++- 2 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 backport-src-groupmod-fix-double-free.patch diff --git a/backport-src-groupmod-fix-double-free.patch b/backport-src-groupmod-fix-double-free.patch new file mode 100644 index 0000000..bc3e4c8 --- /dev/null +++ b/backport-src-groupmod-fix-double-free.patch @@ -0,0 +1,53 @@ +From 10429edc14673fbb8c78b25f1872c34e88e5f07f Mon Sep 17 00:00:00 2001 +From: lixinyun +Date: Wed, 29 May 2024 06:53:02 +0800 +Subject: [PATCH] src/groupmod.c: delete gr_free_members(&grp) to avoid double + free + +Groupmod -U may cause crashes because of double free. If without -a, the first free of (*ogrp).gr_mem is in gr_free_members(&grp), and then in gr_update without -n or gr_remove with -n. +Considering the minimal impact of modifications on existing code, delete gr_free_members(&grp) to avoid double free.Although this may seem reckless, the second free in two different positions will definitely be triggered, and the following two test cases can be used to illustrate the situation : + +[root@localhost src]# ./useradd u1 +[root@localhost src]# ./useradd u2 +[root@localhost src]# ./useradd u3 +[root@localhost src]# ./groupadd -U u1,u2,u3 g1 +[root@localhost src]# ./groupmod -n g2 -U u1,u2 g1 +Segmentation fault + +This case would free (*ogrp).gr_mem in gr_free_members(&grp) due to assignment statements grp = *ogrp, then in if (nflg && (gr_remove (group_name) == 0)), which finally calls gr_free_members(grent) to free (*ogrp).gr_mem again. + +[root@localhost src]# ./useradd u1 +[root@localhost src]# ./useradd u2 +[root@localhost src]# ./useradd u3 +[root@localhost src]# ./groupadd -U u1,u2,u3 g1 +[root@localhost src]# ./groupmod -U u1,u2 g1 +Segmentation fault + +The other case would free (*ogrp).gr_mem in gr_free_members(&grp) too, then in if (gr_update (&grp) == 0), which finally calls gr_free_members(grent) too to free (*ogrp).gr_mem again. + +So the first free is unnecessary, maybe we can drop it. + +Fixes: 342c934a3590 ("add -U option to groupadd and groupmod") +Closes: +Link: +Link: +Link: +Cc: "Serge E. Hallyn" +Reviewed-by: Alejandro Colomar +Signed-off-by: lixinyun +--- + src/groupmod.c | 2 -- + 1 file changed, 2 deletions(-) + +diff -urN a/src/groupmod.c b/src/groupmod.c +--- a/src/groupmod.c 2025-05-26 09:50:31.522156493 +0800 ++++ b/src/groupmod.c 2025-05-26 11:51:32.200081678 +0800 +@@ -269,8 +269,6 @@ + + if (!aflg) { + // requested to replace the existing groups +- if (NULL != grp.gr_mem[0]) +- gr_free_members(&grp); + grp.gr_mem = (char **)xmalloc(sizeof(char *)); + grp.gr_mem[0] = (char *)0; + } else { diff --git a/shadow.spec b/shadow.spec index 4d21f1d..f4cff2e 100644 --- a/shadow.spec +++ b/shadow.spec @@ -1,6 +1,6 @@ Name: shadow Version: 4.9 -Release: 17 +Release: 18 Epoch: 2 License: BSD and GPLv2+ Summary: Tools for managing accounts and shadow password files @@ -100,6 +100,7 @@ Patch80: backport-src-useradd-free-string.patch Patch81: backport-src-useradd.c-get_groups-Fix-memory-leak.patch Patch82: backport-src-gpasswd-Clear-password-in-more-cases.patch Patch83: backport-lib-encrypt.c-Do-not-exit-in-error-case.patch +Patch84: backport-src-groupmod-fix-double-free.patch BuildRequires: gcc, libselinux-devel, audit-libs-devel, libsemanage-devel BuildRequires: libacl-devel, libattr-devel @@ -269,6 +270,9 @@ rm -f $RPM_BUILD_ROOT/%{_libdir}/libsubid.la %{_mandir}/*/* %changelog +* Mon May 26 2025 lixinyun - 2:4.9-18 +- Remove gr_free_members to fix double-free in groupmod -U + * Tue Mar 18 2025 wangjiang - 2:4.9-17 - backport patches from upstream -- Gitee