When we are execing a setugid program, and we have a procfs filesystem

file open in one of the special file descriptors (0, 1, or 2), close
it before completing the exec.

Submitted by: nergal@idea.avet.com.pl
Constructive comments: deraadt@openbsd.org, sef, peter, jkh

3 files changed, 61 insertions, 0 deletions

diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index 26e0cfc..84864a1 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -40,6 +40,7 @@
  */
 
 #include "opt_compat.h"
+#include "opt_dontuse.h"
 #include <sys/param.h>
 #include <sys/systm.h>
 #include <sys/sysproto.h>
@@ -937,6 +938,64 @@ fdfree(p)
 }
 
 /*
+ * For setuid/setgid programs we don't want to people to use that setuidness
+ * to generate error messages which write to a file which otherwise would
+ * otherwise be off limits to the proces.
+ *
+ * This is a gross hack to plug the hole.  A better solution would involve
+ * a special vop or other form of generalized access control mechanism.  We
+ * go ahead and just reject all procfs file systems accesses as dangerous.
+ *
+ * Since setugidsafety calls this only for fd 0, 1 and 2, this check is
+ * sufficient.  We also don't for setugidness since we know we are.
+ */
+static int
+is_unsafe(struct file *fp)
+{
+#if PROCFS
+	if (fp->f_type == DTYPE_VNODE && 
+	    ((struct vnode *)(fp->f_data))->v_tag == VT_PROCFS)
+		return (1);
+#endif
+	return (0);
+}
+
+/*
+ * Make this setguid thing safe, if at all possible.
+ */
+void
+setugidsafety(p)
+	struct proc *p;
+{
+	struct filedesc *fdp = p->p_fd;
+	struct file **fpp;
+	char *fdfp;
+	register int i;
+
+	/* Certain daemons might not have file descriptors. */
+	if (fdp == NULL)
+		return;
+
+	fpp = fdp->fd_ofiles;
+	fdfp = fdp->fd_ofileflags;
+	for (i = 0; i <= fdp->fd_lastfile; i++, fpp++, fdfp++) {
+		if (i > 2)
+			break;
+		if (*fpp != NULL && is_unsafe(*fpp)) {
+			if (*fdfp & UF_MAPPED)
+				(void) munmapfd(p, i);
+			(void) closef(*fpp, p);
+			*fpp = NULL;
+			*fdfp = 0;
+			if (i < fdp->fd_freefile)
+				fdp->fd_freefile = i;
+		}
+	}
+	while (fdp->fd_lastfile > 0 && fdp->fd_ofiles[fdp->fd_lastfile] == NULL)
+		fdp->fd_lastfile--;
+}
+
+/*
  * Close any files on exec?
  */
 void
diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c
index cceadb1..ba88695 100644
--- a/sys/kern/kern_exec.c
+++ b/sys/kern/kern_exec.c
@@ -289,6 +289,7 @@ interpret:
 		if (attr.va_mode & VSGID)
 			p->p_ucred->cr_gid = attr.va_gid;
 		setsugid(p);
+		setugidsafety(p);
 	} else {
 		if (p->p_ucred->cr_uid == p->p_cred->p_ruid &&
 		    p->p_ucred->cr_gid == p->p_cred->p_rgid)
diff --git a/sys/sys/filedesc.h b/sys/sys/filedesc.h
index d383c1f..43630ba 100644
--- a/sys/sys/filedesc.h
+++ b/sys/sys/filedesc.h
@@ -140,6 +140,7 @@ pid_t	fgetown __P((struct sigio *));
 int	fsetown __P((pid_t, struct sigio **));
 void	funsetown __P((struct sigio *));
 void	funsetownlst __P((struct sigiolst *));
+void	setugidsafety __P((struct proc *p));
 #endif
 
 #endif