Fix a multitude of security bugs in the iBCS2 emulator:

- Return NULL instead of returning memory outside of the stackgap
  in stackgap_alloc() (FreeBSD-SA-00:42.linux)
- Check for stackgap_alloc() returning NULL in ibcs2_emul_find();
  other calls to stackgap_alloc() have not been changed since they
  are small fixed-size allocations.
- Replace use of strcpy() with strlcpy() in exec_coff_imgact()
  to avoid buffer overflow
- Use strlcat() instead of strcat() to avoid a one byte buffer
  overflow in ibcs2_setipdomainname()
- Use copyinstr() instead of copyin() in ibcs2_setipdomainname()
  to ensure that the string is null-terminated
- Avoid integer overflow in ibcs2_setgroups() and ibcs2_setgroups()
  by checking that gidsetsize argument is non-negative and
  no larger than NGROUPS_MAX.
- Range-check signal numbers in ibcs2_wait(), ibcs2_sigaction(),
  ibcs2_sigsys() and ibcs2_kill() to avoid accessing array past
  the end (or before the start)

6 files changed, 42 insertions, 12 deletions

diff --git a/sys/i386/ibcs2/ibcs2_misc.c b/sys/i386/ibcs2/ibcs2_misc.c
index 8cd3488..2f8a37f 100644
--- a/sys/i386/ibcs2/ibcs2_misc.c
+++ b/sys/i386/ibcs2/ibcs2_misc.c
@@ -170,12 +170,24 @@ ibcs2_wait(td, uap)
 		if(error)
 		  return error;
 
-		/* convert status/signal result */
-		if(WIFSTOPPED(status))
+		/*
+		 * Convert status/signal result. We must validate the
+		 * signal number stored in the exit status in case
+		 * the user changed it between wait4()'s copyout()
+		 * and our copyin().
+		 */
+		if (WIFSTOPPED(status)) {
+			if (WSTOPSIG(status) <= 0 ||
+			    WSTOPSIG(status) > IBCS2_SIGTBLSZ)
+				return (EINVAL);
 			status =
 			  IBCS2_STOPCODE(bsd_to_ibcs2_sig[_SIG_IDX(WSTOPSIG(status))]);
-		else if(WIFSIGNALED(status))
+		} else if (WIFSIGNALED(status)) {
+			if (WTERMSIG(status) <= 0 ||
+			    WTERMSIG(status) > IBCS2_SIGTBLSZ)
+				return (EINVAL);
 			status = bsd_to_ibcs2_sig[_SIG_IDX(WTERMSIG(status))];
+		}
 		/* else exit status -- identical */
 
 		/* record result/status */
@@ -647,6 +659,10 @@ ibcs2_getgroups(td, uap)
 	gid_t *gp;
 	caddr_t sg = stackgap_init();
 
+	if (uap->gidsetsize < 0)
+		return (EINVAL);
+	if (uap->gidsetsize > NGROUPS_MAX)
+		uap->gidsetsize = NGROUPS_MAX;
 	sa.gidsetsize = uap->gidsetsize;
 	if (uap->gidsetsize) {
 		sa.gidset = stackgap_alloc(&sg, NGROUPS_MAX *
@@ -679,6 +695,8 @@ ibcs2_setgroups(td, uap)
 	gid_t *gp;
 	caddr_t sg = stackgap_init();
 
+	if (uap->gidsetsize < 0 || uap->gidsetsize > NGROUPS_MAX)
+		return (EINVAL);
 	sa.gidsetsize = uap->gidsetsize;
 	sa.gidset = stackgap_alloc(&sg, sa.gidsetsize *
 					    sizeof(gid_t *));
diff --git a/sys/i386/ibcs2/ibcs2_signal.c b/sys/i386/ibcs2/ibcs2_signal.c
index c22831a..6a4b9e4 100644
--- a/sys/i386/ibcs2/ibcs2_signal.c
+++ b/sys/i386/ibcs2/ibcs2_signal.c
@@ -206,6 +206,8 @@ ibcs2_sigaction(td, uap)
 		nbsap = &nbsa;
 	} else
 		nbsap = NULL;
+	if (uap->sig <= 0 || uap->sig > IBCS2_NSIG)
+		return (EINVAL);
 	error = kern_sigaction(td, ibcs2_to_bsd_sig[_SIG_IDX(uap->sig)], &nbsa,
 	    &obsa, 0);
 	if (error == 0 && uap->oact != NULL) {
@@ -222,15 +224,16 @@ ibcs2_sigsys(td, uap)
 {
 	struct proc *p = td->td_proc;
 	struct sigaction sa;
-	int signum = ibcs2_to_bsd_sig[_SIG_IDX(IBCS2_SIGNO(uap->sig))];
+	int signum = IBCS2_SIGNO(uap->sig);
 	int error;
 
-	if (signum <= 0 || signum >= IBCS2_NSIG) {
+	if (signum <= 0 || signum > IBCS2_NSIG) {
 		if (IBCS2_SIGCALL(uap->sig) == IBCS2_SIGNAL_MASK ||
 		    IBCS2_SIGCALL(uap->sig) == IBCS2_SIGSET_MASK)
 			td->td_retval[0] = (int)IBCS2_SIG_ERR;
 		return EINVAL;
 	}
+	signum = ibcs2_to_bsd_sig[_SIG_IDX(signum)];
 	
 	switch (IBCS2_SIGCALL(uap->sig)) {
 	case IBCS2_SIGSET_MASK:
@@ -430,6 +433,8 @@ ibcs2_kill(td, uap)
 {
 	struct kill_args ka;
 
+	if (uap->signo <= 0 || uap->signo > IBCS2_NSIG)
+		return (EINVAL);
 	ka.pid = uap->pid;
 	ka.signum = ibcs2_to_bsd_sig[_SIG_IDX(uap->signo)];
 	return kill(td, &ka);
diff --git a/sys/i386/ibcs2/ibcs2_socksys.c b/sys/i386/ibcs2/ibcs2_socksys.c
index 231b37f..21d29b0 100644
--- a/sys/i386/ibcs2/ibcs2_socksys.c
+++ b/sys/i386/ibcs2/ibcs2_socksys.c
@@ -187,8 +187,10 @@ ibcs2_setipdomainname(td, uap)
 	if ( ptr != NULL ) {
 		ptr++;
 		*ptr = '\0';
-	} else
-		strcat(hname, ".");
+	} else {
+		if (strlcat(hname, ".", sizeof(hname)) >= sizeof(hname))
+			return (EINVAL);
+	}
 
 	/* Set ptr to the end of the string so we can append to it */
 	hlen = strlen(hname);
@@ -197,7 +199,7 @@ ibcs2_setipdomainname(td, uap)
                 return EINVAL;
 
 	/* Append the ipdomain to the end */
-	error = copyin((caddr_t)uap->ipdomainname, ptr, uap->len);
+	error = copyinstr((caddr_t)uap->ipdomainname, ptr, uap->len, NULL);
 	if (error)
 		return (error);
 
diff --git a/sys/i386/ibcs2/ibcs2_util.c b/sys/i386/ibcs2/ibcs2_util.c
index e1ed3ee..d0eaa37 100644
--- a/sys/i386/ibcs2/ibcs2_util.c
+++ b/sys/i386/ibcs2/ibcs2_util.c
@@ -162,8 +162,10 @@ ibcs2_emul_find(td, sgp, prefix, path, pbuf, cflag)
 		*pbuf = buf;
 	else {
 		sz = &ptr[len] - buf;
-		*pbuf = stackgap_alloc(sgp, sz + 1);
-		error = copyout(buf, *pbuf, sz);
+		if ((*pbuf = stackgap_alloc(sgp, sz + 1)) != NULL)
+			error = copyout(buf, *pbuf, sz);
+		else
+			error = ENAMETOOLONG;
 		free(buf, M_TEMP);
 	}
 
diff --git a/sys/i386/ibcs2/ibcs2_util.h b/sys/i386/ibcs2/ibcs2_util.h
index 7c26748..e95e50f 100644
--- a/sys/i386/ibcs2/ibcs2_util.h
+++ b/sys/i386/ibcs2/ibcs2_util.h
@@ -68,7 +68,10 @@ stackgap_alloc(sgp, sz)
 	size_t   sz;
 {
 	void	*p = (void *) *sgp;
-	*sgp += ALIGN(sz);
+	sz = ALIGN(sz);
+	if (*sgp + sz > (caddr_t)(PS_STRINGS - szsigcode))
+		return NULL;
+	*sgp += sz;
 	return p;
 }
 
diff --git a/sys/i386/ibcs2/imgact_coff.c b/sys/i386/ibcs2/imgact_coff.c
index a5080ff..f3a8e96 100644
--- a/sys/i386/ibcs2/imgact_coff.c
+++ b/sys/i386/ibcs2/imgact_coff.c
@@ -388,7 +388,7 @@ exec_coff_imgact(imgp)
 
 			libbuf = malloc(MAXPATHLEN + emul_path_len,
 					M_TEMP, M_WAITOK);
-			strcpy(libbuf, ibcs2_emul_path);
+			strlcpy(libbuf, ibcs2_emul_path, MAXPATHLEN);
 
 		    	for (j = off; j < scns[i].s_size + off;) {
 				long stroff, nextoff;