Try again. (this time it's tested).

Add proc_cansugid that checks if a process may raise it's privileges.
Rework exec to remove the old sugid workaround and check proc_cansugid
just before raising privileges.

3 files changed, 31 insertions, 21 deletions

diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c
index 38438d2b027..2194904adeb 100644
--- a/sys/kern/kern_exec.c
+++ b/sys/kern/kern_exec.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_exec.c,v 1.52 2001/06/22 14:14:08 deraadt Exp $	*/
+/*	$OpenBSD: kern_exec.c,v 1.53 2001/06/22 23:55:23 art Exp $	*/
 /*	$NetBSD: kern_exec.c,v 1.75 1996/02/09 18:59:28 christos Exp $	*/
 
 /*-
@@ -129,18 +129,9 @@ check_exec(p, epp)
 		error = EACCES;
 		goto bad1;
 	}
-	if ((vp->v_mount->mnt_flag & MNT_NOSUID) ||
-	    (p->p_flag & P_TRACED) || p->p_fd->fd_refcnt > 1)
-		epp->ep_vap->va_mode &= ~(VSUID | VSGID);
 
-	/*
-	 * Set the P_SUID* flags early so that we won't be fiddled with when
-	 * we sleep later in this code.
-	 * XXX - this could give us a few false positives and the caller must
-	 *       make sure to save and restore the flags if exec fails.
-	 */
-	if (epp->ep_vap->va_mode & (VSUID|VSGID))
-		p->p_flag |= P_SUGID|P_SUGIDEXEC;
+	if ((vp->v_mount->mnt_flag & MNT_NOSUID))
+		epp->ep_vap->va_mode &= ~(VSUID | VSGID);
 
 	/* check access.  for root we have to see if any exec bit on */
 	if ((error = VOP_ACCESS(vp, VEXEC, p->p_ucred, p)) != 0)
@@ -253,7 +244,6 @@ sys_execve(p, v, retval)
 	char **tmpfap;
 	int szsigcode;
 	extern struct emul emul_native;
-	int saved_sugid;
 
 	/*
 	 * figure out the maximum size of an exec header, if necessary.
@@ -284,7 +274,6 @@ sys_execve(p, v, retval)
 	pack.ep_emul = &emul_native;
 	pack.ep_flags = 0;
 
-	saved_sugid = p->p_flag & (P_SUGID|P_SUGIDEXEC);
 	/* see if we can run it. */
 	if ((error = check_exec(p, &pack)) != 0) {
 		goto freehdr;
@@ -496,11 +485,14 @@ sys_execve(p, v, retval)
 
 	/*
 	 * deal with set[ug]id.
-	 * MNT_NOEXEC and P_TRACED have already been used to disable s[ug]id.
+	 * MNT_NOEXEC has already been used to disable s[ug]id.
 	 */
-	if ((attr.va_mode & (VSUID | VSGID))) {
+	if ((attr.va_mode & (VSUID | VSGID)) && proc_cansugid(p)) {
 		int i;
 
+		p->p_flag |= P_SUGID;
+		p->p_flag |= P_SUGIDEXEC;
+
 #ifdef KTRACE
 		/*
 		 * If process is being ktraced, turn off - unless
@@ -516,8 +508,6 @@ sys_execve(p, v, retval)
 			p->p_ucred->cr_uid = attr.va_uid;
 		if (attr.va_mode & VSGID)
 			p->p_ucred->cr_gid = attr.va_gid;
-		p->p_flag |= P_SUGID;
-		p->p_flag |= P_SUGIDEXEC;
 
 		/*
 		 * For set[ug]id processes, a few caveats apply to
@@ -657,7 +647,6 @@ bad:
 
 freehdr:
 	free(pack.ep_hdr, M_EXEC);
-	p->p_flag = (p->p_flag & ~(P_SUGID|P_SUGIDEXEC)) | saved_sugid;
 	return (error);
 
 exec_abort:
diff --git a/sys/kern/kern_prot.c b/sys/kern/kern_prot.c
index 65b3474d264..b9d15d1a797 100644
--- a/sys/kern/kern_prot.c
+++ b/sys/kern/kern_prot.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_prot.c,v 1.17 2001/06/22 14:14:09 deraadt Exp $	*/
+/*	$OpenBSD: kern_prot.c,v 1.18 2001/06/22 23:55:24 art Exp $	*/
 /*	$NetBSD: kern_prot.c,v 1.33 1996/02/09 18:59:42 christos Exp $	*/
 
 /*
@@ -53,6 +53,7 @@
 #include <sys/timeb.h>
 #include <sys/times.h>
 #include <sys/malloc.h>
+#include <sys/filedesc.h>
 
 #include <sys/mount.h>
 #include <sys/syscallargs.h>
@@ -655,3 +656,21 @@ sys_setlogin(p, v, retval)
 		error = EINVAL;
 	return (error);
 }
+
+/*
+ * Check if a process is allowed to raise its privileges.
+ */
+int
+proc_cansugid(struct proc *p)
+{
+	/* ptrace(2)d processes shouldn't. */
+	if ((p->p_flag & P_TRACED) != 0)
+		return (0);
+
+	/* proceses with shared filedescriptors shouldn't. */
+	if (p->p_fd->fd_refcnt > 1)
+		return (0);
+
+	/* Allow. */
+	return (1);
+}
diff --git a/sys/sys/proc.h b/sys/sys/proc.h
index ae6690314b1..53497a42baf 100644
--- a/sys/sys/proc.h
+++ b/sys/sys/proc.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: proc.h,v 1.42 2001/06/19 07:54:37 deraadt Exp $	*/
+/*	$OpenBSD: proc.h,v 1.43 2001/06/22 23:55:22 art Exp $	*/
 /*	$NetBSD: proc.h,v 1.44 1996/04/22 01:23:21 christos Exp $	*/
 
 /*-
@@ -392,5 +392,7 @@ int	groupmember __P((gid_t, struct ucred *));
 void	cpu_switch __P((struct proc *));
 void	cpu_wait __P((struct proc *));
 void	cpu_exit __P((struct proc *));
+
+int	proc_cansugid __P((struct proc *));
 #endif	/* _KERNEL */
 #endif	/* !_SYS_PROC_H_ */