kern/155321: imgact_shell integer underflow when argv[0] is longer than interp + path

[Devon H. O'Dell]

>Number:         155321
>Category:       kern
>Synopsis:       imgact_shell integer underflow when argv[0] is longer than interp + path
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Sun Mar 06 20:00:18 UTC 2011
>Closed-Date:
>Last-Modified:
>Originator:     Devon H. O'Dell
>Release:        8-STABLE
>Organization:
>Environment:
FreeBSD bigdisk.dho.apt 8.2-STABLE FreeBSD 8.2-STABLE #8: Sun Mar  6 14:20:26 EST 2011     root at bigdisk.dho.apt:/usr/src/sys/amd64/compile/BIGDISK  amd64

>Description:
In debugging a problem with Go calling execve(2), it appears that the actual bug is in the FreeBSD kernel implementation in imgact_shell. Specifically, it is entirely valid for argv[0] to be longer than the length of the path (treated as "fname" in this function). We have a workaround in Go, but this should be fixed in the kernel.
>How-To-Repeat:
Russ Cox wrote a quick to demonstrate the issue:

$ cat execve.c
#include <unistd.h>
#include <stdio.h>

int
main(int argc, char **argv, char **environ)
{
       execve(argv[1], argv+2, environ);
       perror("execve");
       return 1;
}
$ cat shellscript
#!/bin/echo
$ gcc execve.c
$ cp /bin/ls .
$ ./a.out ls /bogus/ls
a.out           execve.c        ls              shellscript
$ ./a.out shellscript asdf
shellscript
$ ./a.out shellscript /bogus/shellscript
shellscript
$ ./a.out shellscript /bin/echo-shellscript
shellscript
$ ./a.out shellscript /bin/echo-shellscript1
execve: Argument list too long

>Fix:
Patch attached.

Patch attached with submission follows:

Index: sys/kern/imgact_shell.c
===================================================================
--- sys/kern/imgact_shell.c	(revision 219345)
+++ sys/kern/imgact_shell.c	(working copy)
@@ -195,16 +195,19 @@
 	length = (imgp->args->argc == 0) ? 0 :
 	    strlen(imgp->args->begin_argv) + 1;		/* bytes to delete */
 
-	if (offset - length > imgp->args->stringspace) {
-		if (sname != NULL)
-			sbuf_delete(sname);
-		return (E2BIG);
+	if (offset >= length) {
+		if (offset - length > imgp->args->stringspace) {
+			if (sname != NULL)
+				sbuf_delete(sname);
+			return (E2BIG);
+		}
+
+		bcopy(imgp->args->begin_argv + length, imgp->args->begin_argv + offset,
+		    imgp->args->endp - (imgp->args->begin_argv + length));
+
+		offset -= length;		/* calculate actual adjustment */
 	}
 
-	bcopy(imgp->args->begin_argv + length, imgp->args->begin_argv + offset,
-	    imgp->args->endp - (imgp->args->begin_argv + length));
-
-	offset -= length;		/* calculate actual adjustment */
 	imgp->args->begin_envv += offset;
 	imgp->args->endp += offset;
 	imgp->args->stringspace -= offset;


>Release-Note:
>Audit-Trail:
>Unformatted: