Date: Fri, 15 Mar 2002 19:02:04 -0800 From: "Crist J. Clark" <crist.clark@attbi.com> To: Dag-Erling Smorgrav <des@ofug.org> Cc: audit@freebsd.org Subject: Re: Preventing chpass(1) DoS Message-ID: <20020315190204.L29705@blossom.cjclark.org> In-Reply-To: <xzpwuwdrjj3.fsf@flood.ping.uio.no>; from des@ofug.org on Fri, Mar 15, 2002 at 03:29:52PM %2B0100 References: <20020315042007.K29705@blossom.cjclark.org> <xzpwuwdrjj3.fsf@flood.ping.uio.no>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Mar 15, 2002 at 03:29:52PM +0100, Dag-Erling Smorgrav wrote:
> "Crist J. Clark" <cjc@freebsd.org> 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 <pw_util.h>
#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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020315190204.L29705>