Subject: lib/4891: NITPICK: skeyverify should do getpriority before setpriority
To: None <gnats-bugs@gnats.netbsd.org>
From: Chris Jones <cjones@clydesdale.math.montana.edu>
List: netbsd-bugs
Date: 01/25/1998 17:20:02
>Number:         4891
>Category:       lib
>Synopsis:       NITPICK: skeyverify should do getpriority before setpriority
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    lib-bug-people (Library Bug People)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Jan 25 16:35:01 1998
>Last-Modified:
>Originator:     Chris Jones
>Organization:
-------------------------------------------------------------------------------
Chris Jones                                    cjones@rupert.honors.montana.edu
           Mad scientist in training...
"Is this going to be a stand-up programming session, sir, or another bug hunt?"
>Release:        <NetBSD-current source date>1.3
>Environment:
	
System: NetBSD clydesdale.math.montana.edu 1.3 NetBSD 1.3 (CLYDESDALE) #0: Wed Jan 7 17:06:26 MST 1998 cjones@clydesdale.math.montana.edu:/usr/newsrc/sys/arch/i386/compile/CLYDESDALE i386


>Description:
skeyverify() in lib/libskey/skeylogin.c does a setpriority to -4 before it
writes its files, in order to decrease the window of opportunity for a would-be
hacker.  (The skeykeys file should be updated quickly, so a single password can
only be used once, not twice.)  When it's done, this function does a
setpriority back to 0.

Theoretically, a sysadmin could have their users log in with a priority other
than 0 by changing something in gettytab.  However, a knowledgeable user could
defeat this by logging in via S/key instead of other means, thus getting his
priority reset to 0 when skeyverify() is done with its work.
	
>How-To-Repeat:
	
>Fix:
*** skeylogin.c.old	Mon Jan 19 13:59:19 1998
--- skeylogin.c	Sun Jan 25 17:10:19 1998
***************
*** 184,189 ****
--- 184,190 ----
  	struct tm *tm;
  	char tbuf[27];
  	char *cp;
+ 	int prevprio;
  
  	time(&now);
  	tm = localtime(&now);
***************
*** 214,226 ****
  	 * to the system
  	 */
  
! 	setpriority(PRIO_PROCESS, 0, -4);
  
  	/* reread the file record NOW*/
  
  	fseek(mp->keyfile,mp->recstart,0);
  	if (fgets(mp->buf,sizeof(mp->buf),mp->keyfile) != mp->buf){
! 		setpriority(PRIO_PROCESS, 0, 0);
  		fclose(mp->keyfile);
  		return -1;
  	}
--- 215,237 ----
  	 * to the system
  	 */
  
! 	/* Save the priority for later use. */
! 	errno = 0;
! 	prevprio = getpriority(PRIO_PROCESS, 0);
! 	if(errno) {
! 		/* Don't report the error; just don't use it later. */
! 		prevprio = PRIO_MAX + 1;
! 	} else {
! 		setpriority(PRIO_PROCESS, 0, -4);
! 	}
  
  	/* reread the file record NOW*/
  
  	fseek(mp->keyfile,mp->recstart,0);
  	if (fgets(mp->buf,sizeof(mp->buf),mp->keyfile) != mp->buf){
! 		if(prevprio != PRIO_MAX + 1) {
! 			setpriority(PRIO_PROCESS, 0, prevprio);
! 		}
  		fclose(mp->keyfile);
  		return -1;
  	}
***************
*** 237,243 ****
  
  	if (memcmp(filekey,fkey,8) != 0){
  		/* Wrong response */
! 		setpriority(PRIO_PROCESS, 0, 0);
  		fclose(mp->keyfile);
  		return 1;
  	}
--- 248,256 ----
  
  	if (memcmp(filekey,fkey,8) != 0){
  		/* Wrong response */
! 		if(prevprio != PRIO_MAX + 1) {
! 			setpriority(PRIO_PROCESS, 0, prevprio);
! 		}
  		fclose(mp->keyfile);
  		return 1;
  	}
***************
*** 255,261 ****
  
  	fclose(mp->keyfile);
  	
! 	setpriority(PRIO_PROCESS, 0, 0);
  	return 0;
  }
  
--- 268,276 ----
  
  	fclose(mp->keyfile);
  	
! 	if(prevprio != PRIO_MAX + 1) {
! 		setpriority(PRIO_PROCESS, 0, prevprio);
! 	}
  	return 0;
  }
  
>Audit-Trail:
>Unformatted: