>Number:         2085
>Category:       bin
>Synopsis:       ypbind uses xid and binding struct pointer interchangeably.
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    bin-bug-people (Utility Bug People)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Feb 16 08:35:14 1996
>Last-Modified:
>Originator:     Chris G. Demetriou
>Organization:
Kernel Hackers 'r' Us
>Release:        NetBSD-current, Feb. 15. 1996
>Environment:
System: NetBSD sun-lamp.pc.cs.cmu.edu 1.1A NetBSD 1.1A (SUN_LAMP) #34: Thu Jan 18 20:53:30 EST 1996 cgd@sun-lamp.pc.cs.cmu.edu:/usr/src/sys/arch/i386/compile/SUN_LAMP i386


>Description:
	ypbind(8) treats RPC transaction IDs (xids) and domain binding
	structure pointers interchangeably.  In particular:
		(1) it uses the domain binding structure's pointer as
		    the transaction IDs for outgoing messages, and
		(2) assumes that the transaction IDs in incoming messages
		    are valid pointers to domain binding structures.

	This logic has several flaws:
		(1) xids are 32 bits, and not all systems use 32-bit
		    pointers.
		(2) if somebody sends ypbind a message with bogus data,
		    it will crash.

>How-To-Repeat:
	Read the ypbind sources, and note that it uses transaction IDs
	right off the network as pointers.  Note the nasty hack used to
	make ypbind work on NetBSD/Alpha.

	Create a program to send bogus data to ypbind.  Watch it crash.

>Fix:
	A patch is included below which fixes the problem.  It also
	fixed the RCS Id's on ypbind to be in the 'standard' NetBSD
	format.

	This patch is based on a patch sent to me by Theo de Raadt on
	April 21, 1995, which he gave me permission to distribute.  I
	fixed it to work properly on NetBSD/Alpha, added code to it to
	uniquify the generated xids, and cleaned it up a bit.

Index: usr.sbin/ypbind/ypbind.c
===================================================================
RCS file: /a/cvsroot/src/usr.sbin/ypbind/ypbind.c,v
retrieving revision 1.19
diff -c -r1.19 ypbind.c
*** ypbind.c	1995/04/21 04:40:36	1.19
--- ypbind.c	1996/02/16 02:58:35
***************
*** 1,3 ****
--- 1,5 ----
+ /*	$NetBSD$	*/
+ 
  /*
   * Copyright (c) 1992, 1993 Theo de Raadt <deraadt@fsa.ca>
   * All rights reserved.
***************
*** 31,37 ****
   */
  
  #ifndef LINT
! static char rcsid[] = "$Id: ypbind.c,v 1.19 1995/04/21 04:40:36 cgd Exp $";
  #endif
  
  #include <sys/param.h>
--- 33,39 ----
   */
  
  #ifndef LINT
! static char rcsid[] = "$NetBSD$";
  #endif
  
  #include <sys/param.h>
***************
*** 76,81 ****
--- 78,84 ----
  	time_t dom_ask_t;
  	int dom_lockfd;
  	int dom_alive;
+ 	int dom_xid;
  };
  
  extern bool_t xdr_domainname(), xdr_ypbind_resp();
***************
*** 99,104 ****
--- 102,110 ----
  u_long rmtcr_port;
  SVCXPRT *udptransp, *tcptransp;
  
+ struct _dom_binding *xid2ypdb __P((int xid));
+ int unique_xid __P((struct _dom_binding *ypdb));
+ 
  void *
  ypbindproc_null_2(transp, argp, clnt)
  	SVCXPRT *transp;
***************
*** 138,143 ****
--- 144,150 ----
  		ypdb->dom_lockfd = -1;
  		sprintf(path, "%s/%s.%d", BINDINGDIR, ypdb->dom_domain, ypdb->dom_vers);
  		unlink(path);
+ 		ypdb->dom_xid = unique_xid(ypdb);
  		ypdb->dom_pnext = ypbindlist;
  		ypbindlist = ypdb;
  		check++;
***************
*** 473,479 ****
  	msg.rm_call.cb_cred = rpcua->ah_cred;
  	msg.rm_call.cb_verf = rpcua->ah_verf;
  
! 	msg.rm_xid = (long)dom;
  	xdrmem_create(&xdr, buf, sizeof buf, XDR_ENCODE);
  	if (!xdr_callmsg(&xdr, &msg)) {
  		st = RPC_CANTENCODEARGS;
--- 480,486 ----
  	msg.rm_call.cb_cred = rpcua->ah_cred;
  	msg.rm_call.cb_verf = rpcua->ah_verf;
  
! 	msg.rm_xid = ypdb->dom_xid;
  	xdrmem_create(&xdr, buf, sizeof buf, XDR_ENCODE);
  	if (!xdr_callmsg(&xdr, &msg)) {
  		st = RPC_CANTENCODEARGS;
***************
*** 538,544 ****
  	msg.rm_call.cb_cred = rpcua->ah_cred;
  	msg.rm_call.cb_verf = rpcua->ah_verf;
  
! 	msg.rm_xid = (long)dom;
  	xdrmem_create(&xdr, buf, sizeof buf, XDR_ENCODE);
  	if (!xdr_callmsg(&xdr, &msg)) {
  		st = RPC_CANTENCODEARGS;
--- 545,551 ----
  	msg.rm_call.cb_cred = rpcua->ah_cred;
  	msg.rm_call.cb_verf = rpcua->ah_verf;
  
! 	msg.rm_xid = ypdb->dom_xid;
  	xdrmem_create(&xdr, buf, sizeof buf, XDR_ENCODE);
  	if (!xdr_callmsg(&xdr, &msg)) {
  		st = RPC_CANTENCODEARGS;
***************
*** 643,648 ****
--- 650,656 ----
  {
  	char buf[1400];
  	int fromlen, inlen;
+ 	struct _dom_binding *ypdb;
  	struct sockaddr_in raddr;
  	struct rpc_msg msg;
  	XDR xdr;
***************
*** 675,681 ****
  		if ((msg.rm_reply.rp_stat == MSG_ACCEPTED) &&
  		    (msg.acpted_rply.ar_stat == SUCCESS)) {
  			raddr.sin_port = htons((u_short)rmtcr_port);
! 			rpc_received(msg.rm_xid, &raddr, 0);
  		}
  	}
  	xdr.x_op = XDR_FREE;
--- 683,691 ----
  		if ((msg.rm_reply.rp_stat == MSG_ACCEPTED) &&
  		    (msg.acpted_rply.ar_stat == SUCCESS)) {
  			raddr.sin_port = htons((u_short)rmtcr_port);
! 			ypdb = xid2ypdb(msg.rm_xid);
! 			if (ypdb != NULL)
! 				rpc_received(ypdb->dom_domain, &raddr, 0);
  		}
  	}
  	xdr.x_op = XDR_FREE;
***************
*** 690,695 ****
--- 700,706 ----
  {
  	char buf[1400];
  	int fromlen, inlen;
+ 	struct _dom_binding *ypdb;
  	struct sockaddr_in raddr;
  	struct rpc_msg msg;
  	XDR xdr;
***************
*** 722,728 ****
  	if (xdr_replymsg(&xdr, &msg)) {
  		if ((msg.rm_reply.rp_stat == MSG_ACCEPTED) &&
  		    (msg.acpted_rply.ar_stat == SUCCESS)) {
! 			rpc_received(msg.rm_xid, &raddr, 0);
  		}
  	}
  	xdr.x_op = XDR_FREE;
--- 733,741 ----
  	if (xdr_replymsg(&xdr, &msg)) {
  		if ((msg.rm_reply.rp_stat == MSG_ACCEPTED) &&
  		    (msg.acpted_rply.ar_stat == SUCCESS)) {
! 			ypdb = xid2ypdb(msg.rm_xid);
! 			if (ypdb != NULL)
! 				rpc_received(ypdb->dom_domain, &raddr, 0);
  		}
  	}
  	xdr.x_op = XDR_FREE;
***************
*** 746,760 ****
  	char path[MAXPATHLEN];
  	int fd;
  
- 	/* XXX XXX USING POINTERS OVER THE NET LIKE THIS IS TOTALLY WRONG. */
- 
  	/*printf("returned from %s about %s\n", inet_ntoa(raddrp->sin_addr), dom);*/
  
- #ifdef __alpha__						/* XXX XXX */
- 	/* you _REALLY_ do not want to know... */		/* XXX XXX */
- 	dom = (char *)((u_long)dom | (u_long)0x100000000);	/* XXX XXX */
- #endif								/* XXX XXX */
- 
  	if (dom == NULL)
  		return;
  
--- 759,766 ----
***************
*** 831,834 ****
--- 837,865 ----
  		ypdb->dom_lockfd = -1;
  		return;
  	}
+ }
+ 
+ struct _dom_binding *
+ xid2ypdb(xid)
+ 	int xid;
+ {
+ 	struct _dom_binding *ypdb;
+ 
+ 	for (ypdb = ypbindlist; ypdb; ypdb = ypdb->dom_pnext)
+ 		if (ypdb->dom_xid == xid)
+ 			break;
+ 	return (ypdb);
+ }
+ 
+ int
+ unique_xid(ypdb)
+ 	struct _dom_binding *ypdb;
+ {
+ 	int tmp_xid;
+ 
+ 	tmp_xid = (long)ypdb & 0xffffffff;
+ 	while (xid2ypdb(tmp_xid) != NULL)
+ 		tmp_xid++;
+ 
+ 	return tmp_xid;
  }
>Audit-Trail:
>Unformatted: