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:
[EMAIL PROTECTED]:~/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
--- loginutils/adduser.c.orig 2007-10-27 08:57:23.000000000 +0200
+++ loginutils/adduser.c 2007-10-27 15:54:25.000000000 +0200
@@ -20,11 +20,6 @@
{
enum { min = 500, max = 65000 };
FILE *passwd;
- /* We are using reentrant fgetpwent_r() in order to avoid
- * pulling in static buffers from libc (think static build here) */
- char buffer[256];
- struct passwd pw;
- struct passwd *result;
passwd = xfopen(filename, "r");
@@ -32,39 +27,29 @@
if ((p->pw_uid > max) || (p->pw_uid < min))
p->pw_uid = min;
- /* stuff to do:
- * make sure login isn't taken;
- * find free uid and gid;
- */
- while (!fgetpwent_r(passwd, &pw, buffer, sizeof(buffer), &result)) {
- if (strcmp(pw.pw_name, p->pw_name) == 0) {
- /* return 0; */
- return 1;
- }
- if ((pw.pw_uid >= p->pw_uid) && (pw.pw_uid < max)
- && (pw.pw_uid >= min)) {
- p->pw_uid = pw.pw_uid + 1;
- }
- }
-
- if (p->pw_gid == 0) {
- /* EDR check for an already existing gid */
- while (getgrgid(p->pw_uid) != NULL)
- p->pw_uid++;
-
- /* EDR also check for an existing group definition */
- if (getgrnam(p->pw_name) != NULL)
+ /* 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;
-
- /* EDR create new gid always = uid */
+ /* 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 */
- if ((p->pw_uid > max) || (p->pw_uid < min))
+ /* EDR bounds check, could not be less than min */
+ if (p->pw_uid > max)
return 2;
- /* return 1; */
return 0;
}
_______________________________________________
busybox mailing list
[email protected]
http://busybox.net/cgi-bin/mailman/listinfo/busybox
