From 722d90bf02ed10d4107fdda355ddd758807c114a Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Fri, 5 Jun 2026 19:11:19 +0000 Subject: [PATCH] fix: drop supplemental groups on _uid usage --- CHANGELOG.md | 4 ++++ sh.py | 6 ++++-- 2 files changed, 8 insertions(+), 2 deletions(-) Index: sh-2.2.2/sh.py =================================================================== --- sh-2.2.2.orig/sh.py +++ sh-2.2.2/sh.py @@ -1826,8 +1826,9 @@ class OProc: pwrec = pwd.getpwuid(ca["uid"]) target_gid = pwrec.pw_gid + target_name = pwrec.pw_name else: - target_uid, target_gid = None, None + target_uid, target_gid, target_name = None, None, None # I had issues with getting 'Input/Output error reading stdin' from dd, # until I set _tty_out=False @@ -2076,6 +2077,7 @@ class OProc: setwinsize(1, ca["tty_size"]) if ca["uid"] is not None: + os.initgroups(target_name, target_gid) os.setgid(target_gid) os.setuid(target_uid) Index: sh-2.2.2/pyproject.toml =================================================================== --- sh-2.2.2.orig/pyproject.toml +++ sh-2.2.2/pyproject.toml @@ -56,3 +56,8 @@ toml = "^0.10.2" [build-system] requires = ["poetry-core>=1.0.0a5"] build-backend = "poetry.core.masonry.api" + +[tool.pytest.ini_options] +markers = [ + "root: marks tests that require root privileges (deselected when not root)", +] Index: sh-2.2.2/tests/sh_test.py =================================================================== --- sh-2.2.2.orig/tests/sh_test.py +++ sh-2.2.2/tests/sh_test.py @@ -23,6 +23,8 @@ from io import BytesIO, StringIO from os.path import dirname, exists, join, realpath, split from pathlib import Path +import pytest + import sh THIS_DIR = Path(__file__).resolve().parent @@ -119,6 +121,7 @@ def requires_progs(*progs): requires_posix = unittest.skipUnless(os.name == "posix", "Requires POSIX") +requires_root = unittest.skipUnless(os.getuid() == 0, "Requires root") requires_utf8 = unittest.skipUnless( sh.DEFAULT_ENCODING == "UTF-8", "System encoding must be UTF-8" ) @@ -3207,6 +3210,73 @@ sys.exit(1) else: self.assertEqual(p.exit_code, -sig) + @requires_posix + @requires_root + @pytest.mark.root + @requires_progs("useradd", "userdel", "groupadd", "groupdel", "id") + def test_uid_drops_supplementary_groups(self): + """Verify that _uid resets supplementary groups to the target user's + own groups via initgroups, not the calling process's groups. + + Regression test for the security issue where a child launched with + _uid= still inherited root's supplementary groups. + """ + import re + + # High IDs to avoid conflicts with real system users/groups. + test_uid = 64001 + test_gid = 64001 + test_group = "sh_test_grp" + test_user = "sh_test_usr" + + group_created = False + user_created = False + try: + sh.groupadd("-g", str(test_gid), test_group) + group_created = True + + # -M: no home dir; -g: set primary group to test_group + sh.useradd("-u", str(test_uid), "-g", test_group, "-M", test_user) + user_created = True + + # Use the `id` utility: it's a world-accessible standard binary + # that reports uid and groups without requiring any Python + # interpreter or tmp file accessible to the low-privilege user. + id_str = str(sh.id(_uid=test_uid)).strip() + + uid_found = int(re.search(r"uid=(\d+)", id_str).group(1)) + self.assertEqual(uid_found, test_uid) + + gid_found = int(re.search(r"gid=(\d+)", id_str).group(1)) + self.assertEqual(gid_found, test_gid) + + groups_part = id_str.split("groups=", 1)[1] if "groups=" in id_str else "" + child_groups = {int(m) for m in re.findall(r"(\d+)", groups_part)} + + # Child must carry the test user's own group. + self.assertIn( + test_gid, + child_groups, + f"Child groups {child_groups} missing expected gid {test_gid}", + ) + # Child must NOT have inherited root's primary group (gid 0). + self.assertNotIn( + 0, + child_groups, + f"Child inherited root's group 0; groups were {child_groups}", + ) + finally: + if user_created: + try: + sh.userdel(test_user) + except Exception: + pass + if group_created: + try: + sh.groupdel(test_group) + except Exception: + pass + class MockTests(BaseTests): def test_patch_command_cls(self):