Skip site navigation (1)Skip section navigation (2)
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>