[Pkg-shadow-devel] Bug#264879: passwd: useradd allows invalid characters as username

[Nicolas François]

--/9DWx/yDrRhgMJTb
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

tags 264879 patch
retitle 264879 [POST-SARGE] useradd: Colon allowed as username
thanks

On Mon, Dec 20, 2004 at 04:04:52AM +0700, David Garamond wrote:
> Not only does it allow colon, but also characters like newline which
> will corrupt the /etc/passwd database. Characters like '/' can also be
> considered invalid, since it will be impossible for the user to have a
> home directory with its name.

It's even worse: good_name does not return 0 on error.  So no checking
is performed.

I'm quite uncomfortable with this bug.  It may be more severe than normal,
but I also fear that fixing it now may break something else.  So the fix
should probably be delayed post Sarge.
A discussion should happen on usernames (e.g. are non-ASCII usernames
allowed?)

Currently, adduser uses a very restrictive regex ("^[a-z][-a-z0-9]*\$";
which can be disabled with --force-badname) and useradd doesn't check
anything.


Attached are:
  * a patch to fix good_name with the proposal from David (no ':', '/' or
    '\n').  There is still a lot of questionable usernames allowed
    (names containing a space or tab, starting by a digit, etc.)
  * the implementation from RedHat, which seems reasonable to me.  (maybe
    we should not allow usernames starting with a '-').
    useradd will still be much more permissive than adduser, but some
    reasonable checks will be performed.
    The RedHat equivalent regex is "^[a-zA-Z_][a-zA-Z0-9_-.]*\$?$"


Just for the record: good_name is used to check username (in useradd,
groupmod and pwck), and groupnames (in groupadd, groupmod and grpck).

Best Regards,
-- 
Nekral

--/9DWx/yDrRhgMJTb
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=redhat-good_name

static int
good_name(const char *name)
{
	/*
	 * User/group names must match [a-zA-Z_][a-zA-Z0-9_-.]*
	 */
	if (!*name || !((*name >= 'a' && *name <= 'z')
                         || (*name >= 'A' && *name <= 'Z') || *name == '_'))
		return 0;

	while (*++name) {
		if (!((*name >= 'a' && *name <= 'z') ||
		    (*name >= 'A' && *name <= 'Z') ||
		    (*name >= '0' && *name <= '9') ||
		    *name == '_' || *name == '-' || *name == '.' ||
		    (*name == '$' && *(name+1) == NULL)))
			return 0;
	}

	return 1;
}

--/9DWx/yDrRhgMJTb
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="chkname.c.patch"

--- chkname.c.orig	2005-03-30 23:07:08.000000000 +0200
+++ chkname.c	2005-03-31 00:08:32.000000000 +0200
@@ -37,11 +37,14 @@
 	}
 #endif
 	/* seeing no sufficiently good reason for the above... */
-	if (*name == '-')
-		return 1;
-	while (*++name)
-		if (*name == ':')
-			return 1;
+	if (*name == '\0' || *name == '-')
+		return 0;
+	/*
+	 * ':' and '\n' will break /etc/passwd
+	 * with '/' the home directory will be an issue
+	 */
+	if (strcspn (name, ":\n/") != strlen (name))
+		return 0;
 
 	return 1;
 }

--/9DWx/yDrRhgMJTb--