Synopsis: race between sugid-exec and ptrace(2)
NetBSD versions: 1.5, -current
Thanks to: Jason Thorpe <thorpej@netbsd.org>
Reported in NetBSD Security Advisory: NetBSD-SA2001-009

Index: sys/compat/netbsd32/netbsd32_execve.c
===================================================================
RCS file: /cvsroot/syssrc/sys/compat/netbsd32/netbsd32_execve.c,v
retrieving revision 1.3
retrieving revision 1.4
diff -p -c -p -r1.3 -r1.4
*** sys/compat/netbsd32/netbsd32_execve.c	2001/05/30 11:37:28	1.3
--- sys/compat/netbsd32/netbsd32_execve.c	2001/06/15 17:24:20	1.4
*************** netbsd32_execve2(p, uap, retval)
*** 371,380 ****
  
  	/*
  	 * deal with set[ug]id.
! 	 * MNT_NOSUID and P_TRACED have already been used to disable s[ug]id.
  	 */
! 	if (((attr.va_mode & S_ISUID) != 0 && p->p_ucred->cr_uid != attr.va_uid)
! 	 || ((attr.va_mode & S_ISGID) != 0 && p->p_ucred->cr_gid != attr.va_gid)){
  		p->p_ucred = crcopy(cred);
  #ifdef KTRACE
  		/*
--- 371,391 ----
  
  	/*
  	 * deal with set[ug]id.
! 	 * MNT_NOSUID has already been used to disable s[ug]id.
  	 */
! 	if ((p->p_flag & P_TRACED) == 0 &&
! 
! 	    (((attr.va_mode & S_ISUID) != 0 &&
! 	      p->p_ucred->cr_uid != attr.va_uid) ||
! 
! 	     ((attr.va_mode & S_ISGID) != 0 &&
! 	      p->p_ucred->cr_gid != attr.va_gid))) {
! 		/*
! 		 * Mark the process as SUGID before we do
! 		 * anything that might block.
! 		 */
! 		p_sugid(p);
! 
  		p->p_ucred = crcopy(cred);
  #ifdef KTRACE
  		/*
*************** netbsd32_execve2(p, uap, retval)
*** 388,394 ****
  			p->p_ucred->cr_uid = attr.va_uid;
  		if (attr.va_mode & S_ISGID)
  			p->p_ucred->cr_gid = attr.va_gid;
- 		p_sugid(p);
  	} else
  		p->p_flag &= ~P_SUGID;
  	p->p_cred->p_svuid = p->p_ucred->cr_uid;
--- 399,404 ----
Index: sys/kern/exec_script.c
===================================================================
RCS file: /cvsroot/syssrc/sys/kern/exec_script.c,v
retrieving revision 1.28
retrieving revision 1.29
diff -p -c -p -r1.28 -r1.29
*** sys/kern/exec_script.c	2001/06/14 20:32:47	1.28
--- sys/kern/exec_script.c	2001/06/15 17:24:19	1.29
*************** exec_script_makecmds(struct proc *p, str
*** 143,150 ****
  check_shell:
  #ifdef SETUIDSCRIPTS
  	/*
! 	 * MNT_NOSUID and STRC are already taken care of by check_exec,
! 	 * so we don't need to worry about them now or later.
  	 */
  	script_sbits = epp->ep_vap->va_mode & (S_ISUID | S_ISGID);
  	if (script_sbits != 0) {
--- 143,151 ----
  check_shell:
  #ifdef SETUIDSCRIPTS
  	/*
! 	 * MNT_NOSUID has already taken care of by check_exec,
! 	 * so we don't need to worry about it now or later.  We
! 	 * will need to check P_TRACED later, however.
  	 */
  	script_sbits = epp->ep_vap->va_mode & (S_ISUID | S_ISGID);
  	if (script_sbits != 0) {
*************** check_shell:
*** 260,266 ****
  #ifdef SETUIDSCRIPTS
  		/*
  		 * set thing up so that set-id scripts will be
! 		 * handled appropriately
  		 */
  		epp->ep_vap->va_mode |= script_sbits;
  		if (script_sbits & S_ISUID)
--- 261,269 ----
  #ifdef SETUIDSCRIPTS
  		/*
  		 * set thing up so that set-id scripts will be
! 		 * handled appropriately.  P_TRACED will be
! 		 * checked later when the shell is actually
! 		 * exec'd.
  		 */
  		epp->ep_vap->va_mode |= script_sbits;
  		if (script_sbits & S_ISUID)
Index: sys/kern/kern_exec.c
===================================================================
RCS file: /cvsroot/syssrc/sys/kern/kern_exec.c,v
retrieving revision 1.140
retrieving revision 1.141
diff -p -c -p -r1.140 -r1.141
*** sys/kern/kern_exec.c	2001/05/07 09:55:14	1.140
--- sys/kern/kern_exec.c	2001/06/15 17:24:19	1.141
*************** check_exec(struct proc *p, struct exec_p
*** 212,218 ****
  		error = EACCES;
  		goto bad1;
  	}
! 	if ((vp->v_mount->mnt_flag & MNT_NOSUID) || (p->p_flag & P_TRACED))
  		epp->ep_vap->va_mode &= ~(S_ISUID | S_ISGID);
  
  	/* try to open it */
--- 212,218 ----
  		error = EACCES;
  		goto bad1;
  	}
! 	if (vp->v_mount->mnt_flag & MNT_NOSUID)
  		epp->ep_vap->va_mode &= ~(S_ISUID | S_ISGID);
  
  	/* try to open it */
*************** sys_execve(struct proc *p, void *v, regi
*** 591,600 ****
  
  	/*
  	 * deal with set[ug]id.
! 	 * MNT_NOSUID and P_TRACED have already been used to disable s[ug]id.
  	 */
! 	if (((attr.va_mode & S_ISUID) != 0 && p->p_ucred->cr_uid != attr.va_uid)
! 	 || ((attr.va_mode & S_ISGID) != 0 && p->p_ucred->cr_gid != attr.va_gid)){
  		p->p_ucred = crcopy(cred);
  #ifdef KTRACE
  		/*
--- 591,611 ----
  
  	/*
  	 * deal with set[ug]id.
! 	 * MNT_NOSUID has already been used to disable s[ug]id.
  	 */
! 	if ((p->p_flag & P_TRACED) == 0 &&
! 
! 	    (((attr.va_mode & S_ISUID) != 0 &&
! 	      p->p_ucred->cr_uid != attr.va_uid) ||
! 
! 	     ((attr.va_mode & S_ISGID) != 0 &&
! 	      p->p_ucred->cr_gid != attr.va_gid))) {
! 		/*
! 		 * Mark the process as SUGID before we do
! 		 * anything that might block.
! 		 */
! 		p_sugid(p);
! 
  		p->p_ucred = crcopy(cred);
  #ifdef KTRACE
  		/*
*************** sys_execve(struct proc *p, void *v, regi
*** 608,614 ****
  			p->p_ucred->cr_uid = attr.va_uid;
  		if (attr.va_mode & S_ISGID)
  			p->p_ucred->cr_gid = attr.va_gid;
- 		p_sugid(p);
  	} else
  		p->p_flag &= ~P_SUGID;
  	p->p_cred->p_svuid = p->p_ucred->cr_uid;
--- 619,624 ----