Subject: Re: lib/1764: __ivaliduser() contains call to abort()
To: None <gnats-bugs@NetBSD.ORG>
From: John Kohl <jtk@kolvir.arlington.ma.us>
List: netbsd-bugs
Date: 11/16/1995 07:32:41
Actually, there are other stylistic no-nos in rcmd.c (assuming boolean
value of a == b is either 0 or 1), plus an inefficiency (calling
gethostbyaddr() each loop rather than once before the loop).
It's also the case that as currently coded you cannot have a bare IP
address in your .rhosts file and have it work unless there is a PTR
record (or YP entry or /etc/hosts line) for the remote host.

===================================================================
RCS file: RCS/rcmd.c,v
retrieving revision 1.1
diff -c -r1.1 rcmd.c
*** rcmd.c	1995/11/16 03:49:22	1.1
--- rcmd.c	1995/11/16 03:54:02
***************
*** 361,373 ****
  	int ch;
  	char buf[MAXHOSTNAMELEN + 128];		/* host + login */
  	const char *auser, *ahost;
! 	int hostok, userok;
  	char rhost[MAXHOSTNAMELEN];
  	struct hostent *hp;
  	char domain[MAXHOSTNAMELEN];
  
  	getdomainname(domain, sizeof(domain));
  
  	while (fgets(buf, sizeof(buf), hostf)) {
  		p = buf;
  		/* Skip lines that are too long. */
--- 361,381 ----
  	int ch;
  	char buf[MAXHOSTNAMELEN + 128];		/* host + login */
  	const char *auser, *ahost;
! 	int hostok, userok, have_rhost;
  	char rhost[MAXHOSTNAMELEN];
  	struct hostent *hp;
  	char domain[MAXHOSTNAMELEN];
  
  	getdomainname(domain, sizeof(domain));
  
+ 	if ((hp = gethostbyaddr((char *) &raddr,
+ 				sizeof(raddr), AF_INET)) == NULL) {
+ 		have_rhost = 0;
+ 	} else {
+ 		(void) strncpy(rhost, hp->h_name, sizeof(rhost));
+ 		rhost[sizeof(rhost) - 1] = '\0';
+ 		have_rhost = 1;
+ 	}
  	while (fgets(buf, sizeof(buf), hostf)) {
  		p = buf;
  		/* Skip lines that are too long. */
***************
*** 397,409 ****
  		auser = *user ? user : luser;
  		ahost = buf;
  
- 		if ((hp = gethostbyaddr((char *) &raddr,
- 					sizeof(raddr), AF_INET)) == NULL) {
- 			abort();
- 			return -1;
- 		}
- 		(void) strncpy(rhost, hp->h_name, sizeof(rhost));
- 		rhost[sizeof(rhost) - 1] = '\0';
  
  		if (ahost[0] == '+')
  			switch (ahost[1]) {
--- 405,410 ----
***************
*** 412,419 ****
  				break;
  
  			case '@':
! 				hostok = innetgr(&ahost[2], rhost, NULL,
! 						 domain);
  				break;
  
  			default:
--- 413,423 ----
  				break;
  
  			case '@':
! 				if (have_rhost)
! 					hostok = innetgr(&ahost[2], rhost,
! 							 NULL, domain);
! 				else
! 					hostok = 0;
  				break;
  
  			default:
***************
*** 427,434 ****
  				break;
  
  			case '@':
! 				hostok = -innetgr(&ahost[2], rhost, NULL,
! 						  domain);
  				break;
  
  			default:
--- 431,441 ----
  				break;
  
  			case '@':
! 				if (have_rhost)
! 					hostok = -innetgr(&ahost[2], rhost,
! 							  NULL, domain);
! 				else
! 					hostok = 0;
  				break;
  
  			default:
***************
*** 451,457 ****
  				break;
  
  			default:
! 				userok = strcmp(ruser, &auser[1]) == 0;
  				break;
  			}
  		else if (auser[0] == '-')
--- 458,464 ----
  				break;
  
  			default:
! 				userok = strcmp(ruser, &auser[1]) == 0 ? 1 : 0;
  				break;
  			}
  		else if (auser[0] == '-')
***************
*** 466,476 ****
  				break;
  
  			default:
! 				userok = -(strcmp(ruser, &auser[1]) == 0);
  				break;
  			}
  		else
! 			userok = strcmp(ruser, auser) == 0;
  
  		/* Check if one component did not match */
  		if (hostok == 0 || userok == 0)
--- 473,483 ----
  				break;
  
  			default:
! 				userok = strcmp(ruser, &auser[1]) == 0 ? -1 : 0;
  				break;
  			}
  		else
! 			userok = strcmp(ruser, auser) == 0 ? 1 : 0;
  
  		/* Check if one component did not match */
  		if (hostok == 0 || userok == 0)