kern/134010: [gssapi][patch] Buffer overflow and use-after-free in gssd_syscall

[Mateusz Guzik]

>Number:         134010
>Category:       kern
>Synopsis:       [gssapi][patch] Buffer overflow and use-after-free in gssd_syscall
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Sat Apr 25 23:20:01 UTC 2009
>Closed-Date:
>Last-Modified:
>Originator:     Mateusz Guzik
>Release:        8.0-CURRENT
>Organization:
>Environment:
FreeBSD eternal 8.0-CURRENT FreeBSD 8.0-CURRENT #1: Sat Apr 25 17:21:50 CEST 2009     f at eternal:/usr/obj/srv/build/CURRENT/src/sys/ETERNAL  i386

>Description:
1) Buffer overflow

gssd_syscall contains the following code:

char path[MAXPATHLEN];
[..]
error = copyinstr(uap->path, path, sizeof(path), NULL);
[..]
strcpy(sun.sun_path, path);

sun_path's size is 104 while MAXPATHLEN expands to 1024, thus providing string large enough will cause buffer overflow.

2) Use after free

error = priv_check(td, PRIV_NFS_DAEMON);
if (error)
    return (error);

if (kgss_gssd_handle)
    CLNT_DESTROY(kgss_gssd_handle);

error = copyinstr(uap->path, path, sizeof(path), NULL);
if (error)
    return (error);
[..]
kgss_gssd_handle = clnt_reconnect_create(nconf,[..]

So one "correct" call of gssd_syscall will set kgss_gssd_handle, the first call with incorrect path will invalidate it and the second call will cause panic.
>How-To-Repeat:
Using the following code:
int
main(int argc, char **argv)
{
	gssd_syscall(argv[1]);
	return (0);
}


1) Buffer overflow
./a.out `perl -e 'print("A"x1000)'`

2) Use after free

./a.out `perl -e 'print("A"x100)'`; ./a.out `perl -e 'print("A"x2000)'`; ./a.out `perl -e 'print("A"x2000)'`

>Fix:
Replace MAXPATHLEN with sizeof(sun.sun_path) and move CLNT_DESTROY(kgss_gssd_handle) after copyinstr.


Patch attached with submission follows:

--- gss_impl.c.orig	2008-11-03 11:38:00.000000000 +0100
+++ gss_impl.c	2009-04-26 00:42:01.000000000 +0200
@@ -89,20 +89,20 @@
 {
         struct sockaddr_un sun;
         struct netconfig *nconf;
-	char path[MAXPATHLEN];
+	char path[sizeof(sun.sun_path)];
 	int error;
         
 	error = priv_check(td, PRIV_NFS_DAEMON);
 	if (error)
 		return (error);
 
-	if (kgss_gssd_handle)
-		CLNT_DESTROY(kgss_gssd_handle);
-
 	error = copyinstr(uap->path, path, sizeof(path), NULL);
 	if (error)
 		return (error);
 
+	if (kgss_gssd_handle)
+		CLNT_DESTROY(kgss_gssd_handle);
+
         sun.sun_family = AF_LOCAL;
         strcpy(sun.sun_path, path);
         sun.sun_len = SUN_LEN(&sun);


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