From nobody Fri Aug 15 05:04:23 2025 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4c3953578Fz64Y5W; Fri, 15 Aug 2025 05:04:23 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R10" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4c39533zrlz3v1g; Fri, 15 Aug 2025 05:04:23 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1755234263; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=4COs3v/n8KUmm5UyxKDWhjkykJDsqpgFQnZfln9VZpI=; b=jW8LimKniQL1OrJiggrcoZQfHeM5Q6tuH/cWLlM+G+aKMH9yqQMT4/PfT7VvmQLMZ3FVKm YO9S80Gzt92dPKp+NsiRdKFQC3DuxobISf3pfUOL/9pEbPTdV2ZaRPHoQmp0HXeFnlW46v Dd6mx44efAfBmIPerzcXKYItir5qCXyTthrKWuSAFZ0votKE2ScdExqRy63aYvOBcRh9Vw nwN60SxTClBt0kDAP6ERmLrDXA1uIJwgWxWj1T6i9GJRTWXD4WyFl8p4HYjDP8kYSbIOMK EysvPae0VmKNApVF99VMs2OYucnEO6HcMvMfu5UswhELB0QCPolZ0/63kLfx3w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1755234263; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=4COs3v/n8KUmm5UyxKDWhjkykJDsqpgFQnZfln9VZpI=; b=WZ6NLOcnoEK37cexID3hm7kXnTaZEqfsXE8K2wgWqWTyoXt7jZsacpk2cy54ZThvOZy9Mg ejfydqcgFXp6QZcO87gKr3pkcp1qcRjZmwaxabauffCidikgjvtqOi0okPNlQZy5087KUJ Aqzsplou6knkcDxDnUTWOw84PoImnOTmp/eKGBMfZzyibjaJswKGSCfpTN6LQt4bPCW2UW vBDoWw68HqkOGzBG6Zl7I8AHuMEvRFa+EhcdYHdNzlFZlev5EhT0rI7F/EFSPq28iixzYc Dis1N/0LPLMe2jiTVWft8a4L4pG4+FzD5ql6Xei6oo8LjsUnBNn2xIP514y4JQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1755234263; a=rsa-sha256; cv=none; b=ch7uZo2gazU9Jm8jM5X3ndhQVOoclSpXp5CFKQRgpqSIDIhdZn/wdb+DEu9ZGljOrO95Nv xNeSvO5nNcHjSkltjeiPYHK/TXZg0lPJjiLlEzNikm2lVA66BbAUlz13wTUOY3VoXGbWXN s/EzB+8gAGj4EeKO7c1m5XA0J8Xrsfnq7Q0MyT2iusk9J3G8W9KiqWxT1ygqf9P5MH/fj/ 7KYubkmLTlw399QFRbmqO/jTcRRfgs8xUd/Af4Kx4ymUb5x55/R5LIFZJ8Y+hkeSw0NWqV 2CqmJsJnfaFiOn5T08CT/JsZ6znkhhzxJSe3PbDIHeCb/6OJhQlyJ6UQ7lfUmg== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4c39533Y4qz8J1; Fri, 15 Aug 2025 05:04:23 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 57F54Nod045838; Fri, 15 Aug 2025 05:04:23 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 57F54NLx045835; Fri, 15 Aug 2025 05:04:23 GMT (envelope-from git) Date: Fri, 15 Aug 2025 05:04:23 GMT Message-Id: <202508150504.57F54NLx045835@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Kyle Evans Subject: git: c96e754ea688 - stable/13 - chroot: don't clobber the egid with the first supplemental group List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kevans X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: c96e754ea6884f5b8969861e6df66f4a33dad638 Auto-Submitted: auto-generated The branch stable/13 has been updated by kevans: URL: https://cgit.FreeBSD.org/src/commit/?id=c96e754ea6884f5b8969861e6df66f4a33dad638 commit c96e754ea6884f5b8969861e6df66f4a33dad638 Author: Kyle Evans AuthorDate: 2025-07-26 06:11:58 +0000 Commit: Kyle Evans CommitDate: 2025-08-15 05:03:24 +0000 chroot: don't clobber the egid with the first supplemental group There are two problems here, really: 1.) If -G is specified, the egid of the runner will get clobbered by the first supplemental group 2.) If both -G and -g are specified, the first supplemental group will get clobbered by the -g group Ideally our users shouldn't have to understand the quirks of our setgroups(2) and the manpage doesn't describe the group list as needing to contain the egid, so populate the egid slot as necessary. I note that this code seems to have already been marginally aware of the historical behavior because it was allocating NGROUPS_MAX + 1, but this is an artifact of a later conversion to doing dynamic allocations instead of pushing NGROUPS_MAX arrays on the stack -- the original code did in-fact only have an NGROUPS_MAX-sized array, and the layout was still incorrect. Reviewed by: olce (cherry picked from commit 48fd05999b0f8e822fbf7069779378d103a35f5c) (cherry picked from commit babab49eee9472f628d774996de13d13d296c8c0) --- usr.sbin/chroot/chroot.c | 57 ++++++++++++++++++++++++++++++------------------ 1 file changed, 36 insertions(+), 21 deletions(-) diff --git a/usr.sbin/chroot/chroot.c b/usr.sbin/chroot/chroot.c index 208be32f85ab..b762157046d6 100644 --- a/usr.sbin/chroot/chroot.c +++ b/usr.sbin/chroot/chroot.c @@ -73,7 +73,9 @@ main(int argc, char *argv[]) gid = 0; uid = 0; + gids = 0; user = group = grouplist = NULL; + gidlist = NULL; nonprivileged = false; while ((ch = getopt(argc, argv, "G:g:u:n")) != -1) { switch(ch) { @@ -89,6 +91,11 @@ main(int argc, char *argv[]) break; case 'G': grouplist = optarg; + + /* + * XXX Why not allow us to drop all of our supplementary + * groups? + */ if (*grouplist == '\0') usage(); break; @@ -120,29 +127,37 @@ main(int argc, char *argv[]) } } - ngroups_max = sysconf(_SC_NGROUPS_MAX) + 1; - if ((gidlist = malloc(sizeof(gid_t) * ngroups_max)) == NULL) - err(1, "malloc"); - for (gids = 0; - (p = strsep(&grouplist, ",")) != NULL && gids < ngroups_max; ) { - if (*p == '\0') - continue; - - if (isdigit((unsigned char)*p)) { - gidlist[gids] = (gid_t)strtoul(p, &endp, 0); - if (*endp != '\0') - goto getglist; - } else { + if (grouplist != NULL) { + ngroups_max = sysconf(_SC_NGROUPS_MAX) + 1; + if ((gidlist = malloc(sizeof(gid_t) * ngroups_max)) == NULL) + err(1, "malloc"); + /* Populate the egid slot in our groups to avoid accidents. */ + if (gid == 0) + gidlist[0] = getegid(); + else + gidlist[0] = gid; + for (gids = 1; (p = strsep(&grouplist, ",")) != NULL && + gids < ngroups_max; ) { + if (*p == '\0') + continue; + + if (isdigit((unsigned char)*p)) { + gidlist[gids] = (gid_t)strtoul(p, &endp, 0); + if (*endp != '\0') + goto getglist; + } else { getglist: - if ((gp = getgrnam(p)) != NULL) - gidlist[gids] = gp->gr_gid; - else - errx(1, "no such group `%s'", p); + if ((gp = getgrnam(p)) != NULL) + gidlist[gids] = gp->gr_gid; + else + errx(1, "no such group `%s'", p); + } + gids++; } - gids++; + + if (p != NULL && gids == ngroups_max) + errx(1, "too many supplementary groups provided"); } - if (p != NULL && gids == ngroups_max) - errx(1, "too many supplementary groups provided"); if (user != NULL) { if (isdigit((unsigned char)*user)) { @@ -168,7 +183,7 @@ main(int argc, char *argv[]) if (chdir(argv[0]) == -1 || chroot(".") == -1) err(1, "%s", argv[0]); - if (gids && setgroups(gids, gidlist) == -1) + if (gidlist != NULL && setgroups(gids, gidlist) == -1) err(1, "setgroups"); if (group && setgid(gid) == -1) err(1, "setgid");