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: