Fixed security hole with sharing the file descriptor table (via rfork)

when execing a setuid/setgid binary. Code submitted by Sean Eric Fagan
(sef@freebsd.org).
Also consolidated the setuid/setgid checks into one place.
Reviewed by:	dyson,sef

1 files changed, 18 insertions, 10 deletions

diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c
index 4c7f331..3bf808c 100644
--- a/sys/kern/kern_exec.c
+++ b/sys/kern/kern_exec.c
@@ -23,7 +23,7 @@
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  *
- *	$Id: kern_exec.c,v 1.62 1997/04/18 02:43:05 davidg Exp $
+ *	$Id: kern_exec.c,v 1.63 1997/04/23 22:07:05 ache Exp $
  */
 
 #include <sys/param.h>
@@ -257,6 +257,18 @@ interpret:
 	else
 		suword(--stack_base, imgp->argc);
 
+	/*
+	 * For security and other reasons, the file descriptor table cannot
+	 * be shared after an exec.
+	 */
+	if (p->p_fd->fd_refcnt > 1) {
+		struct filedesc *tmp;
+
+		tmp = fdcopy(p);
+		fdfree(p);
+		p->p_fd = tmp;
+	}
+
 	/* close files on exec */
 	fdcloseexec(p);
 
@@ -279,10 +291,13 @@ interpret:
 	}
 
 	/*
-	 * Implement image setuid/setgid. Disallow if the process is
-	 * being traced.
+	 * Implement image setuid/setgid.
+	 *
+	 * Don't honor setuid/setgid if the filesystem prohibits it or if
+	 * the process is being traced.
 	 */
 	if ((attr.va_mode & (VSUID | VSGID)) &&
+	    (imgp->vp->v_mount->mnt_flag & MNT_NOSUID) == 0 &&
 	    (p->p_flag & P_TRACED) == 0) {
 		/*
 		 * Turn off syscall tracing for set-id programs, except for
@@ -649,12 +664,5 @@ exec_check_permissions(imgp)
 	if (error)
 		return (error);
 
-	/*
-	 * Disable setuid/setgid if the filesystem prohibits it or if
-	 * the process is being traced.
-	 */
-        if ((vp->v_mount->mnt_flag & MNT_NOSUID) || (p->p_flag & P_TRACED))
-		attr->va_mode &= ~(VSUID | VSGID);
-
 	return (0);
 }