Subject: bin/3416: /usr/bin/w causes segmentation faults, etc.
To: None <gnats-bugs@gnats.netbsd.org>
From: None <enami@cv.sony.co.jp>
List: netbsd-bugs
Date: 03/31/1997 12:02:34
>Number:         3416
>Category:       bin
>Synopsis:       /usr/bin/w causes segmentation faults, etc.
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    bin-bug-people (Utility Bug People)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Mar 30 19:20:01 1997
>Last-Modified:
>Originator:     enami tsugutomo
>Organization:
	Sony Corp.
>Release:        NetBSD-current 1997 Mar. 30
>Environment:
System: NetBSD parity-error.cv.sony.co.jp 1.2D NetBSD 1.2D (PARITY_ERROR) #139: Mon Mar 31 10:23:45 JST 1997 enami@java-tea.cv.sony.co.jp:/usr/src/sys/arch/i386/compile/PARITY_ERROR i386


>Description:
	(1) /usr/bin/w causes segmentation faults because it overruns
	string constants and tries to modify following string constant
	area.

	(2) In previous version of w.c, local variable x in function
	main() pointed to after ':' if non-null, but in version 1.21,
	it points where ':' exists (actually, it is replaced by '\0').

>How-To-Repeat:
	Type w.  Here is results on the machine I'm using.

enami@parity-error% w
11:22AM  up 31 mins, 5 users, load averages: 0.31, 0.17, 0.15
USER    TTY FROM              LOGIN@  IDLE WHAT
Segmentation fault
enami@parity-error%

>Fix:
	(1) In the following piece of code, when *ep->utmp.ut_host is
	NUL, variable p points strings constant "-".  Obvoiusly, its
	length is 1 rather than UT_HOSTSIZE, so x will overruns (and
	find the ':' of "%s:%.*s").

		p = *ep->utmp.ut_host ? ep->utmp.ut_host : "-";
		for (x = p; x < p + UT_HOSTSIZE; x++)
			if (*x == ':') {
				*x = '\0';
				break;
			}
		if (x == p + UT_HOSTSIZE)
			x = NULL;

	(2) Version 1.20 of w.c has following piece of code instead of
	above:

! 		if ((x = strchr(p, ':')) != NULL)
! 			*x++ = '\0';

	Value of x after the each piece of code are executed is
	different, isn't it?  In former case, x points NUL, but in
	latter case, x points just after NUL.

	Here is my fix for (1) and (2); don't iterate and just set x
	and p to some constant if *ep->utmp.ut_host is NUL, and
	increment x after assigning NUL to *x.

Index: w.c
===================================================================
RCS file: /cvsroot/NetBSD/src/usr.bin/w/w.c,v
retrieving revision 1.1.1.6
diff -c -r1.1.1.6 w.c
*** w.c	1997/03/30 23:57:45	1.1.1.6
--- w.c	1997/03/31 02:46:13
***************
*** 291,304 ****
  		}
  
  	for (ep = ehead; ep != NULL; ep = ep->next) {
! 		p = *ep->utmp.ut_host ? ep->utmp.ut_host : "-";
! 		for (x = p; x < p + UT_HOSTSIZE; x++)
! 			if (*x == ':') {
! 				*x = '\0';
! 				break;
! 			}
! 		if (x == p + UT_HOSTSIZE)
  			x = NULL;
  		if (!nflag && isdigit(*p) &&
  		    (long)(l = inet_addr(p)) != -1 &&
  		    (hp = gethostbyaddr((char *)&l, sizeof(l), AF_INET))) {
--- 291,309 ----
  		}
  
  	for (ep = ehead; ep != NULL; ep = ep->next) {
! 		if (*ep->utmp.ut_host == '\0') {
! 			p = "-";
  			x = NULL;
+ 		} else {
+ 			p = ep->utmp.ut_host;
+ 			for (x = p; x < p + UT_HOSTSIZE; x++)
+ 				if (*x == ':') {
+ 					*x++ = '\0';
+ 					break;
+ 				}
+ 			if (x == p + UT_HOSTSIZE)
+ 				x = NULL;
+ 		}
  		if (!nflag && isdigit(*p) &&
  		    (long)(l = inet_addr(p)) != -1 &&
  		    (hp = gethostbyaddr((char *)&l, sizeof(l), AF_INET))) {
>Audit-Trail:
>Unformatted: