linux-kernel - [PATCH 05/11] exec: In bprm_fill_uid use CAP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87lflcxdwc.fsf_-_@x220.int.ebiederm.org>
Date:   Thu, 28 May 2020 10:44:35 -0500
From:   ebiederm@...ssion.com (Eric W. Biederman)
To:     <linux-kernel@...r.kernel.org>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Oleg Nesterov <oleg@...hat.com>, Jann Horn <jannh@...gle.com>,
        Kees Cook <keescook@...omium.org>,
        Greg Ungerer <gerg@...ux-m68k.org>,
        Rob Landley <rob@...dley.net>,
        Bernd Edlinger <bernd.edlinger@...mail.de>,
        <linux-fsdevel@...r.kernel.org>, Al Viro <viro@...IV.linux.org.uk>,
        Alexey Dobriyan <adobriyan@...il.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Casey Schaufler <casey@...aufler-ca.com>,
        linux-security-module@...r.kernel.org,
        James Morris <jmorris@...ei.org>,
        "Serge E. Hallyn" <serge@...lyn.com>,
        Andy Lutomirski <luto@...capital.net>
Subject: [PATCH 05/11] exec: In bprm_fill_uid use CAP_SETGID to see if a gid change is safe

If the task has CAP_SETGID and a shared fs struct or is being ptraced
than it is clear that nothing new is being introduced when the gid
changes, and so it is safe to honor a setgid executable.

However if all we know is that the task has CAP_SETUID things are less
clear.

This bug looks like it was introduced in v2.1.100 when !suser was
replaced by !capable(CAP_SETUID).  It appears to have been an oversight
at that time.

Fixing this 22 years later seems weird but even now it still looks
worth fixing.  As conceptually what is happening is testing to see if
the process already had the potential to make a gid change or if the
trancer needs permissions in addition to the permissions needed to
trace the process to trace the process through a gid change.

Fixes: v2.1.100
Signed-off-by: "Eric W. Biederman" <ebiederm@...ssion.com>
---
 fs/exec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/exec.c b/fs/exec.c
index 956ee3a0d824..bac8db14f30d 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1654,7 +1654,7 @@ static void bprm_fill_uid(struct linux_binprm *bprm)
 			new->euid = new->uid;
 		}
 		if (need_cap && !gid_eq(new->egid, new->gid) &&
-		    (!ns_capable(new->user_ns, CAP_SETUID) ||
+		    (!ns_capable(new->user_ns, CAP_SETGID) ||
 		     (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS))) {
 			new->egid = new->gid;
 		}
-- 
2.25.0