Bug 29172

Summary: [PATCH] Add more checks in rpc/svc_vc.c and bugfixes
Product: Base System Reporter: mb <mb>
Component: binAssignee: freebsd-bugs (Nobody) <bugs>
Status: Closed FIXED    
Severity: Affects Only Me    
Priority: Normal    
Version: 5.0-CURRENT   
Hardware: Any   
OS: Any   

Description mb 2001-07-23 22:00:01 UTC
Add more security checks for the credential stuff in svc_vc.c in the
rpc library. Fill in the the correct infos into xprt->xp_raddr.

For more info, see the patch with documentation.

Fix: 

Add my patch:

http://home.teleport.ch/freebsd/userland/local_transp_svc_vc.c.diff
Comment 1 mb 2001-10-07 01:42:04 UTC
Ok, here is again a patch instead just a URL ;-)

lib/libc/rpc/svc_vc.c

Finally fix the credential stuff. I had to look at the code a long long
time before I've seen what exactly was the breakage.

In NetBSD, Solaris, xprt->xp_p2 pointed directly to the credentials,
in FreeBSD xprt->xp_verf.oa_base was a pointer to a struct cmessage,
which is defined as follow:

struct cmessage {
        struct cmsghdr cmsg;
        struct cmsgcred cmcred;
};

The credentials were submitted the right way and xprt->xp_p2 pointed to them.
But cb_verf.oa_flavor was still empty. There was an assignment missing
in svc_recv() in svc_vc.c:

msg->rm_call.cb_verf.oa_flavor = AUTH_UNIX;

Also

+       if (addr.ss_family == AF_LOCAL) {
+               xprt->xp_raddr = *(struct sockaddr_in *)xprt->xp_rtaddr.buf;
+               xprt->xp_addrlen = sizeof (struct sockaddr_in);
+       }

was missing. But the first seems not to be needed:

I guess in rpc.yppasswdd there was a typo:

- transp>xp_verf.oa_flavor != AUTH_UNIX) {
+ rqstp->rq_cred.oa_flavor != AUTH_UNIX) {

This little fix does fix the breakage in rpc.yppasswdd :-)

+       if (msg.msg_controllen == 0 ||
+           (msg.msg_flags & MSG_CTRUNC) != 0)
+               return (-1);

We cannot set the cb_verf.oa_length in svc_recv() of svc_vc.c, the credentials
get overwritten then, and that's bad.

diff -ruN lib/libc/rpc/svc_vc.c lib/libc/rpc/svc_vc.c
--- lib/libc/rpc/svc_vc.c.orig	Thu Oct  4 15:11:41 2001
+++ lib/libc/rpc/svc_vc.c	Thu Oct  4 20:55:17 2001
@@ -313,6 +313,10 @@
 		return (FALSE);
 	memcpy(xprt->xp_rtaddr.buf, &addr, len);
 	xprt->xp_rtaddr.len = len;
+	if (addr.ss_family == AF_LOCAL) {
+		xprt->xp_raddr = *(struct sockaddr_in *)xprt->xp_rtaddr.buf;
+		xprt->xp_addrlen = sizeof (struct sockaddr_in);
+	}
 #ifdef PORTMAP
 	if (addr.ss_family == AF_INET) {
 		xprt->xp_raddr = *(struct sockaddr_in *)xprt->xp_rtaddr.buf;
@@ -423,13 +427,15 @@
 		}
 	} while ((pollfd.revents & POLLIN) == 0);

+	cm = NULL;
 	sa = (struct sockaddr *)xprt->xp_rtaddr.buf;
 	if (sa->sa_family == AF_LOCAL) {
 		cm = (struct cmessage *)xprt->xp_verf.oa_base;
 		if ((len = __msgread_withcred(sock, buf, len, cm)) > 0) {
 			xprt->xp_p2 = &cm->cmcred;
 			return (len);
-		}
+		} else
+			goto fatal_err;
 	} else {
 		if ((len = _read(sock, buf, (size_t)len)) > 0)
 			return (len);
@@ -656,7 +663,12 @@
 	ret = _recvmsg(sock, &msg, 0);
 	bcopy(&cm.cmsg, &cmp->cmsg, sizeof(cmp->cmsg));
 	bcopy(CMSG_DATA(&cm), &cmp->cmcred, sizeof(cmp->cmcred));
-	return ret;
+
+	if (msg.msg_controllen == 0 ||
+	   (msg.msg_flags & MSG_CTRUNC) != 0)
+		return (-1);
+
+	return (ret);
 }

 static int
@@ -696,10 +708,21 @@
 __rpc_get_local_uid(SVCXPRT *transp, uid_t *uid)
 {
 	struct cmsgcred *cmcred;
+	struct cmessage *cm;
+	struct cmsghdr *cmp;
+
+	cm = (struct cmessage *)transp->xp_verf.oa_base;
+
+	if (cm == NULL)
+		return (-1);
+	cmp = &cm->cmsg;
+	if (cmp == NULL || cmp->cmsg_level != SOL_SOCKET ||
+	   cmp->cmsg_type != SCM_CREDS)
+		return (-1);

 	cmcred = __svc_getcallercreds(transp);
 	if (cmcred == NULL)
-		return(-1);
+		return (-1);
 	*uid = cmcred->cmcred_euid;
-	return(0);
+	return (0);
 }
Comment 2 Martin Blapp freebsd_committer freebsd_triage 2002-02-09 20:42:07 UTC
State Changed
From-To: open->closed

Alfred comitted my fixes. Thanks