[PATCH] adduser can create duplicate users

[Tito]

Hi,
this patch fixes a problem in adduser spotted by Ralf Friedl:

>Also, I think it is possible that this implementation results in 
>duplicate user ids:
>The first loop, "while (!fgetpwent_r(..." find a free user id.
>The second loop, "while (getgrgid(p->pw_uid)) p->pw_uid++;" increments 
>uid until it finds an unused group id.
>It is possible that this incremented uid is in use as a user id.

The patch is tested and works for me but I would like it to be  reviewed on the list
to avoid some of mine silly errors ;-) .
As side effect the patch also reduces size:

root at localhost:~/Desktop/busybox.latest# scripts/bloat-o-meter busybox_old busybox_unstripped
function                                             old     new   delta
bb_internal_fgetpwent_r                               51       -     -51
adduser_main                                         909     679    -230
------------------------------------------------------------------------------
(add/remove: 0/1 grow/shrink: 0/1 up/down: 0/-281)           Total: -281 bytes

The patch changes the passwd_study function:

static int passwd_study(const char *filename, struct passwd *p)
{
	enum { min = 500, max = 65000 };
	FILE *passwd;

	passwd = xfopen(filename, "r");

	/* EDR if uid is out of bounds, set to min */
	if ((p->pw_uid > max) || (p->pw_uid < min))
		p->pw_uid = min;

	/* check for an already in use login name */
	if (getpwnam(p->pw_name))
		 return 1;

	/* search a free uid */
	while (getpwuid(p->pw_uid))
		p->pw_uid++;

	if (!p->pw_gid) {
		/* check for an already in use group name */
		if (getgrnam(p->pw_name))
			return 3;
		/* search a free gid and free uid with the same value */
		while (getgrgid(p->pw_uid) || getpwuid(p->pw_uid))
			p->pw_uid++;
		/* set gid = uid */
		p->pw_gid = p->pw_uid;
	}

	/* EDR bounds check, could not be less than min */
	if (p->pw_uid > max)
		return 2;

	return 0;
}

Ciao,
Tito
-------------- next part --------------
A non-text attachment was scrubbed...
Name: adduser_fix.patch
Type: text/x-diff
Size: 1812 bytes
Desc: not available
Url : http://busybox.net/lists/busybox/attachments/20071027/daa9d89e/attachment.bin