Subject: bug in rlogind.c
To: None <current-users@sun-lamp.cs.berkeley.edu>
From: Christos Zoulas <christos@deshaw.com>
List: current-users
Date: 05/24/1994 12:46:20
I just found a horrible bug in rlogind.c:

ruserok() now calls gethostbyname(host) with "host" passed from rlogind.c
This "host" can be the result of a hp = gethostbyaddr() call and it is really
host == hp->h_name!

When that happens, and we have a /etc/hosts lookup the same local buffer
(hostbuf) from gethostnameaddr.c is used to place the result, and hp->h_name
is now trashed!

The following fix, simplifies the code too, since remotehost is used to
keep the remote host name or stringified address!

I have not checked any other programs that user ruserok(), but someone should.
Maybe it is worth copying the buffer one extra time in ruserok(), to make
sure that it is not being passed hp->h_name...

christos


*** rlogind.c.dist	Fri May 20 06:27:27 1994
--- rlogind.c	Tue May 24 12:21:05 1994
***************
*** 195,201 ****
  	int authenticated = 0, hostok = 0;
  	register struct hostent *hp;
  	char remotehost[2 * MAXHOSTNAMELEN + 1];
- 	struct hostent hostent;
  	char c;
  
  	alarm(60);
--- 195,200 ----
***************
*** 213,223 ****
  	hp = gethostbyaddr((char *)&fromp->sin_addr, sizeof(struct in_addr),
  		fromp->sin_family);
  	if (hp == 0) {
! 		/*
! 		 * Only the name is used below.
! 		 */
! 		hp = &hostent;
! 		hp->h_name = inet_ntoa(fromp->sin_addr);
  		hostok++;
  	} else if (check_all || local_domain(hp->h_name)) {
  		/*
--- 212,218 ----
  	hp = gethostbyaddr((char *)&fromp->sin_addr, sizeof(struct in_addr),
  		fromp->sin_family);
  	if (hp == 0) {
! 		strcpy(remotehost, inet_ntoa(fromp->sin_addr));
  		hostok++;
  	} else if (check_all || local_domain(hp->h_name)) {
  		/*
***************
*** 233,249 ****
  		    for (; hp->h_addr_list[0]; hp->h_addr_list++)
  			if (!bcmp(hp->h_addr_list[0], (caddr_t)&fromp->sin_addr,
  			    sizeof(fromp->sin_addr))) {
  				hostok++;
  				break;
  			}
! 	} else
  		hostok++;
  
  #ifdef	KERBEROS
  	if (use_kerberos) {
  		if (!hostok)
  			fatal(f, "rlogind: Host address mismatch.", 0);
! 		retval = do_krb_login(hp->h_name, fromp);
  		if (retval == 0)
  			authenticated++;
  		else if (retval > 0)
--- 228,247 ----
  		    for (; hp->h_addr_list[0]; hp->h_addr_list++)
  			if (!bcmp(hp->h_addr_list[0], (caddr_t)&fromp->sin_addr,
  			    sizeof(fromp->sin_addr))) {
+ 				strcpy(remotehost, hp->h_name);
  				hostok++;
  				break;
  			}
! 	} else {
! 		strcpy(remotehost, hp->h_name);
  		hostok++;
+ 	}
  
  #ifdef	KERBEROS
  	if (use_kerberos) {
  		if (!hostok)
  			fatal(f, "rlogind: Host address mismatch.", 0);
! 		retval = do_krb_login(remotehost, fromp);
  		if (retval == 0)
  			authenticated++;
  		else if (retval > 0)
***************
*** 287,293 ****
  	        }
  	    }
  #endif
! 	    if (do_rlogin(hp->h_name) == 0 && hostok)
  		    authenticated++;
  	}
  	if (confirmed == 0) {
--- 285,291 ----
  	        }
  	    }
  #endif
! 	    if (do_rlogin(remotehost) == 0 && hostok)
  		    authenticated++;
  	}
  	if (confirmed == 0) {
***************
*** 324,337 ****
  				syslog(LOG_INFO|LOG_AUTH,
  				    "ROOT Kerberos login from %s.%s@%s on %s\n",
  				    kdata->pname, kdata->pinst, kdata->prealm,
! 				    hp->h_name);
  #endif
  
  			execl(_PATH_LOGIN, "login", "-p",
! 			    "-h", hp->h_name, "-f", lusername, 0);
  		} else
  			execl(_PATH_LOGIN, "login", "-p",
! 			    "-h", hp->h_name, lusername, 0);
  		fatal(STDERR_FILENO, _PATH_LOGIN, 1);
  		/*NOTREACHED*/
  	}
--- 322,335 ----
  				syslog(LOG_INFO|LOG_AUTH,
  				    "ROOT Kerberos login from %s.%s@%s on %s\n",
  				    kdata->pname, kdata->pinst, kdata->prealm,
! 				    remotehost);
  #endif
  
  			execl(_PATH_LOGIN, "login", "-p",
! 			    "-h", remotehost, "-f", lusername, 0);
  		} else
  			execl(_PATH_LOGIN, "login", "-p",
! 			    "-h", remotehost, lusername, 0);
  		fatal(STDERR_FILENO, _PATH_LOGIN, 1);
  		/*NOTREACHED*/
  	}

------------------------------------------------------------------------------