From nobody Fri Aug 15 05:04:19 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 4c394z2ZF5z64Y85; Fri, 15 Aug 2025 05:04:19 +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 4c394z202rz3ty5; Fri, 15 Aug 2025 05:04:19 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1755234259; 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=Y/MzKCv4/NXOgaARZ2FfQXzUUzlXKr5q2WjN1znfIjs=; b=Ce1154STayLwE5UgswyrYrIMzDlO7OfSqCoMEBT6GD4QLJ3fr1NhlI5M0te2H9AIKQp7qO SFByBHvqGykp1tmYPhCfmMiCJsItxKVZMFGGnx1F6pgSq5CjKW78Yd4jlUMeELy3nymmVL 7ejjVmOVnzJBAjjZ1I0RehodujXDTxhGf3t9GB3qGy5Ff9UXqfvzeW9EUfp2r6BrLcuX81 myWPOVT10KSUB6JjHgqQ7u2JgCLu4TXqDwAU6ZvRNhDMHZhAfHtPYJZy6rnrN0aNfYkM+y yz703+Cp0ZcEv9uAZzs3rOzJaanSINRxd5kEAb/XkJZYkk1lZGzYl88HpBWZBg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1755234259; 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=Y/MzKCv4/NXOgaARZ2FfQXzUUzlXKr5q2WjN1znfIjs=; b=i+v1aJWebq1Gs26Gr84brePsTcCnQRAuXCw0ym53+M1Yd7OChLG/XHcPbWKkD8qRuomqiE 4+F0vNqTuoKTBRLhrQTOv9u2qBqVtT3cp7oWUvEqLsrtWtnyPE5qP+gaZyMgOWzDq2mShO Fu6gJowJJz4pFZMzf/4I/67nCCDh1VQ+RX//He943+VZ5x1Uxd/63LEPfpJjofAWzh66/5 8EFM/Eoin+N7yHUDbRT0Hqrmhx8Bps9h5yvpeDbNUyi9foMXKrrAGLEB7+qBMY8CI0voHn u5kwdRtp/MbQ49Xfq/elXqYP4XBCyKEfJci/VwXkMU/ulB6C7xn7UlrYRNuRrg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1755234259; a=rsa-sha256; cv=none; b=Yso7t4PlsBM6W5Fg3qqjVdpg24+w8cTxCiMG6NgRzgTigd8L0u6wVgSt8OxdsvkgGztYyw 8Tky89iFw64v90EIc0/5nXAcJSGErQnvsu2s6wgZLrsBvour80z4ixGR5D6fHwgR3Ws0BU t/LmkAULEMKkxd3D82/X8zm2w0AvCnZueQhQVz3eEYoGUUAkjo8bvoBzhDxT5lVtWJC5uX TRDSGe1ObZ8iuZijPdqZYmMfUPmcOY6uZwegZUWxzoiF602LaCOMQ0FAkiEePG/XVmpSJd yLibGPfYDtlAf1jB1vhPF/VP9PZFejUxIFFeetk1Xy1btvrV9eORu+Vc31/VIg== 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 4c394z1YSnz8J0; Fri, 15 Aug 2025 05:04:19 +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 57F54JRY045703; Fri, 15 Aug 2025 05:04:19 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 57F54JU8045700; Fri, 15 Aug 2025 05:04:19 GMT (envelope-from git) Date: Fri, 15 Aug 2025 05:04:19 GMT Message-Id: <202508150504.57F54JU8045700@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: 3eafc9f76fd5 - stable/14 - 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/14 X-Git-Reftype: branch X-Git-Commit: 3eafc9f76fd54fbd222d587b11752290766a0af5 Auto-Submitted: auto-generated The branch stable/14 has been updated by kevans: URL: https://cgit.FreeBSD.org/src/commit/?id=3eafc9f76fd54fbd222d587b11752290766a0af5 commit 3eafc9f76fd54fbd222d587b11752290766a0af5 Author: Kyle Evans AuthorDate: 2025-07-26 06:11:58 +0000 Commit: Kyle Evans CommitDate: 2025-08-15 05:04:12 +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 9fc918e38eac..b73ac65f5cad 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");