Date: Fri, 15 Mar 2002 04:20:07 -0800 From: "Crist J. Clark" <cjc@freebsd.org> To: audit@freebsd.org Subject: Preventing chpass(1) DoS Message-ID: <20020315042007.K29705@blossom.cjclark.org>
next in thread | raw e-mail | index | archive | help
PR 35816 describes a problem with the current chpass(1) implementation. That is, any user can tie up the password database indefinately by running chpass(1) and just leaving the interactive editor it spawns open. The PR was originally closed stating the fix was for the administrator to hunt down and kill the offending process. I do not think that is a good answer. Between the time someone runs chpass(1) and an administrator is found, gains access to the system, and hunts down the open editor, no one on the system can successfully run chpass(1) or even passwd(1). I personally do not think that is acceptable. I have made some small changes to the code (I chose to make the diffs as small as possible rather than make a larger change which might be more asthetically pleasing) which do not lock the database while the user is editing. Instead of locking the database over the whole process the following steps occur, - Get the struct passwd for the user being modified. - Edit the data for the user. - Lock the password database. - Re-read the struct passwd for the user. - Merge the edits into the struct passwd. - Rebuild database. So why do it this way? If changes are made to the user's database entry while the data is being edited, we will merge the changes with the edited data. The only potential problem I see is when a superuser edits data at the same time as another user and finishes last. Modification made by the other user will be overwritten by the superuser's "changes." It's not a big enough problem to worry me much. I should note that a cursory look at NetBSD and OpenBSD shows they both do not lock while the user is editing. Anyway, the patches follow. Also, would anyone mind if I style(9)ized chpass(1)? Index: usr.bin/chpass/chpass.c =================================================================== RCS file: /export/freebsd/ncvs/src/usr.bin/chpass/chpass.c,v retrieving revision 1.18 diff -u -r1.18 chpass.c --- usr.bin/chpass/chpass.c 26 Jul 2001 23:27:10 -0000 1.18 +++ usr.bin/chpass/chpass.c 15 Mar 2002 11:41:37 -0000 @@ -246,12 +246,11 @@ * The exit closes the master passwd fp/fd. */ pw_init(); - pfd = pw_lock(); tfd = pw_tmp(); if (op == EDITENTRY) { display(tfd, pw); - edit(pw); + edit(&pw, username, &pfd); (void)unlink(tempname); tfd = pw_tmp(); } Index: usr.bin/chpass/chpass.h =================================================================== RCS file: /export/freebsd/ncvs/src/usr.bin/chpass/chpass.h,v retrieving revision 1.3 diff -u -r1.3 chpass.h --- usr.bin/chpass/chpass.h 3 Sep 1998 17:32:22 -0000 1.3 +++ usr.bin/chpass/chpass.h 15 Mar 2002 11:42:21 -0000 @@ -54,7 +54,7 @@ int atot __P((char *, time_t *)); void display __P((int, struct passwd *)); -void edit __P((struct passwd *)); +void edit __P((struct passwd **, const char *username, int *pfd)); char *ok_shell __P((char *)); int p_change __P((char *, struct passwd *, ENTRY *)); int p_class __P((char *, struct passwd *, ENTRY *)); Index: usr.bin/chpass/edit.c =================================================================== RCS file: /export/freebsd/ncvs/src/usr.bin/chpass/edit.c,v retrieving revision 1.19 diff -u -r1.19 edit.c --- usr.bin/chpass/edit.c 26 Jul 2001 23:27:10 -0000 1.19 +++ usr.bin/chpass/edit.c 15 Mar 2002 11:42:12 -0000 @@ -62,8 +62,10 @@ extern char *tempname; void -edit(pw) - struct passwd *pw; +edit(pw, username, pfd) + struct passwd **pw; + const char *username; + int *pfd; { struct stat begin, end; char *begin_sum, *end_sum; @@ -83,8 +85,14 @@ } free(begin_sum); free(end_sum); - if (verify(pw)) + *pfd = pw_lock(); + if ((*pw = getpwnam(username)) == NULL) + errx(1, "user \'%s\' disappeared while editing", + username); + if (verify(*pw)) break; + if (close(*pfd) != 0) + errx(1, "lost lock on password database"); pw_prompt(); } } -- Crist J. Clark | cjclark@alum.mit.edu | cjclark@jhu.edu http://people.freebsd.org/~cjc/ | cjc@freebsd.org To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020315042007.K29705>