From owner-freebsd-audit Fri Mar 15 19: 2:15 2002 Delivered-To: freebsd-audit@freebsd.org Received: from rwcrmhc54.attbi.com (rwcrmhc54.attbi.com [216.148.227.87]) by hub.freebsd.org (Postfix) with ESMTP id 98F9837B405 for ; Fri, 15 Mar 2002 19:02:09 -0800 (PST) Received: from blossom.cjclark.org ([12.234.91.48]) by rwcrmhc54.attbi.com (InterMail vM.4.01.03.27 201-229-121-127-20010626) with ESMTP id <20020316030209.WXYH1214.rwcrmhc54.attbi.com@blossom.cjclark.org>; Sat, 16 Mar 2002 03:02:09 +0000 Received: (from cjc@localhost) by blossom.cjclark.org (8.11.6/8.11.6) id g2G324b46011; Fri, 15 Mar 2002 19:02:04 -0800 (PST) (envelope-from cjc) Date: Fri, 15 Mar 2002 19:02:04 -0800 From: "Crist J. Clark" To: Dag-Erling Smorgrav Cc: audit@freebsd.org Subject: Re: Preventing chpass(1) DoS Message-ID: <20020315190204.L29705@blossom.cjclark.org> Reply-To: cjclark@alum.mit.edu References: <20020315042007.K29705@blossom.cjclark.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i In-Reply-To: ; from des@ofug.org on Fri, Mar 15, 2002 at 03:29:52PM +0100 X-URL: http://people.freebsd.org/~cjc/ Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Fri, Mar 15, 2002 at 03:29:52PM +0100, Dag-Erling Smorgrav wrote: > "Crist J. Clark" writes: > > I should note that a cursory look at NetBSD and OpenBSD shows they > > both do not lock while the user is editing. > > How about using their code rather than roll our own? *shrug* Bigger diff. They also have pw_scan() exposed to the world in libc. We don't. But instead of using pw_scan() here's an approach that does the comparison "backwards" from how they do it. But other than that, it's a pretty straighforward matter to use the same approach. Anyone see any issues with this? 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 16 Mar 2002 02:51:11 -0000 @@ -83,7 +83,7 @@ char **argv; { enum { NEWSH, LOADENTRY, EDITENTRY, NEWPW, NEWEXP } op; - struct passwd *pw = NULL, lpw; + struct passwd *pw = NULL, lpw, old_pw; char *username = NULL; int ch, pfd, tfd; char *arg = NULL; @@ -160,7 +160,7 @@ uid = getuid(); - if (op == EDITENTRY || op == NEWSH || op == NEWPW || op == NEWEXP) + if (op == EDITENTRY || op == NEWSH || op == NEWPW || op == NEWEXP) { switch(argc) { #ifdef YP case 0: @@ -186,6 +186,12 @@ default: usage(); } + + /* Make a copy for later verification */ + old_pw = *pw; + old_pw.pw_gecos = strdup(old_pw.pw_gecos); + } + if (op == NEWSH) { /* protect p_shell -- it thinks NULL is /bin/sh */ if (!arg[0]) @@ -246,7 +252,6 @@ * The exit closes the master passwd fp/fd. */ pw_init(); - pfd = pw_lock(); tfd = pw_tmp(); if (op == EDITENTRY) { @@ -262,7 +267,8 @@ (void)unlink(tempname); } else { #endif /* YP */ - pw_copy(pfd, tfd, pw); + pfd = pw_lock(); + pw_copy(pfd, tfd, pw, (op == LOADENTRY) ? NULL : &old_pw); if (!pw_mkdb(username)) pw_error((char *)NULL, 0, 1); Index: usr.bin/chpass/pw_copy.c =================================================================== RCS file: /export/freebsd/ncvs/src/usr.bin/chpass/pw_copy.c,v retrieving revision 1.10 diff -u -r1.10 pw_copy.c --- usr.bin/chpass/pw_copy.c 26 Jul 2001 23:27:10 -0000 1.10 +++ usr.bin/chpass/pw_copy.c 16 Mar 2002 02:57:01 -0000 @@ -51,20 +51,38 @@ #include #include "pw_copy.h" +#define PWLINE_MAX 8192 +#define PWFIELD_MAX 20 + extern char *tempname; +static char uidstr[PWFIELD_MAX]; +static char gidstr[PWFIELD_MAX]; +static char chgstr[PWFIELD_MAX]; +static char expstr[PWFIELD_MAX]; + +static char * +pw_fmt(char *buf, size_t size, struct passwd *pw) +{ + (void)snprintf(buf, size, "%s:%s:%s:%s:%s:%s:%s:%s:%s:%s\n", + pw->pw_name, pw->pw_passwd, + pw->pw_fields & _PWF_UID ? uidstr : "", + pw->pw_fields & _PWF_GID ? gidstr : "", + pw->pw_class, + pw->pw_fields & _PWF_CHANGE ? chgstr : "", + pw->pw_fields & _PWF_EXPIRE ? expstr : "", + pw->pw_gecos, pw->pw_dir, pw->pw_shell); + return buf; +} + void -pw_copy(ffd, tfd, pw) +pw_copy(ffd, tfd, pw, old_pw) int ffd, tfd; - struct passwd *pw; + struct passwd *pw, *old_pw; { FILE *from, *to; int done; - char *p, buf[8192]; - char uidstr[20]; - char gidstr[20]; - char chgstr[20]; - char expstr[20]; + char *p, buf[PWLINE_MAX], old_buf[PWLINE_MAX]; snprintf(uidstr, sizeof(uidstr), "%lu", (unsigned long)pw->pw_uid); snprintf(gidstr, sizeof(gidstr), "%lu", (unsigned long)pw->pw_gid); @@ -108,14 +126,13 @@ goto err; continue; } - (void)fprintf(to, "%s:%s:%s:%s:%s:%s:%s:%s:%s:%s\n", - pw->pw_name, pw->pw_passwd, - pw->pw_fields & _PWF_UID ? uidstr : "", - pw->pw_fields & _PWF_GID ? gidstr : "", - pw->pw_class, - pw->pw_fields & _PWF_CHANGE ? chgstr : "", - pw->pw_fields & _PWF_EXPIRE ? expstr : "", - pw->pw_gecos, pw->pw_dir, pw->pw_shell); + *p = ':'; + if (old_pw != NULL && + strcmp(buf, pw_fmt(old_buf, sizeof(old_buf), old_pw)) != 0) { + warnx("%s: entry inconsistent", _PATH_MASTERPASSWD); + pw_error(NULL, 0, 1); + } + (void)fprintf(to, "%s", pw_fmt(buf, sizeof(buf), pw)); done = 1; if (ferror(to)) goto err; Index: usr.bin/chpass/pw_copy.h =================================================================== RCS file: /export/freebsd/ncvs/src/usr.bin/chpass/pw_copy.h,v retrieving revision 1.1.1.1 diff -u -r1.1.1.1 pw_copy.h --- usr.bin/chpass/pw_copy.h 27 May 1994 12:30:55 -0000 1.1.1.1 +++ usr.bin/chpass/pw_copy.h 16 Mar 2002 02:00:19 -0000 @@ -33,4 +33,4 @@ * @(#)pw_copy.h 8.1 (Berkeley) 4/2/94 */ -void pw_copy __P((int, int, struct passwd *)); +void pw_copy __P((int, int, struct passwd *, struct passwd *)); -- 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