enforce maxlength in usernames

[Tito]

On Thursday 28 July 2011 23:36:57 Rich Felker wrote:
> On Thu, Jul 28, 2011 at 10:29:01PM +0200, Matthias Andree wrote:
> > > 	/* For example, name like ".." can make adduser
> > > 	 * chown "/home/.." recursively - NOT GOOD
> > > 	 */
> > > 
> > > 	do {
> > > 		if (*name == '-' || *name == '.')
> > > 			continue;
> > >  skip:
> > > 		if (isalnum(*name)
> > 
> > This is bogus and can lead to segfaults through out-of-bounds array
> > subscripts on systems with signed chars.  This needs to be
> > isalnum((unsigned char)*name).  This is true for all toupper/tolower and
> > is*() functions from <ctype.h> where the argument is as wide as char.
> 
> Any use of isalnum or similar in validating login names is completely
> bogus because it cannot handle alphanumeric multibyte characters.
> Either skip the validation (which is almost surely just harmful and
> not helpful to anyone) or make correct use of mbrtowc and iswalnum.
> 
> Rich

Hi,
after more research about this topic I've found a few interesting things
about this can of worms I've unknowningly uncovered:

1) 3.426 User Name

A string that is used to identify a user; see also User Database. To be portable
across systems conforming to IEEE Std 1003.1-2001, the value is composed
of characters from the portable filename character set. 
The hyphen should not be used as the first character of a portable user name.

(http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap03.html#tag_03_426)

2) 3.213 Login 
The unspecified activity by which a user gains access to the system. Each login is associated with exactly one login name. 
3) 3.214 Login Name
A user name that is associated with a login.
3.276 Portable Filename Character Set
The set of characters from which portable filenames are constructed.

    A B C D E F G H I J K L M N O P Q R S T U V W X Y Z
    a b c d e f g h i j k l m n o p q r s t u v w x y z
    0 1 2 3 4 5 6 7 8 9 . _ -

4) the adduser (and probably addgroup) programs in most debian
    based systems do in fact check the user (or group) name to be created
    for illegal characters with a configurable regex (and a default hardcoded one
    shown below). It is evident that is almost identic with  Portable Filename Character Set.

sub checkname {
    my ($name) = @_;
    if ($name !~ /^[_.A-Za-z0-9][-\@_.A-Za-z0-9]*\$?$/) {
        printf STDERR
(gtx("%s: To avoid problems, the username should consist only of
letters, digits, underscores, periods, at signs and dashes, and not start with
a dash (as defined by IEEE Std 1003.1-2001). For compatibility with Samba
machine accounts \$ is also supported at the end of the username\n"), $0);
        exit RET_INVALID_CHARS_IN_NAME;;
    }

5) the useradd program called by adduser and adgroup programs
    based systems also enforce some checks on username and
   groupname length as stated in man useradd:

        Usernames may only be up to 32 characters long.

sudo adduser qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq
Adding user `qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq' ...
Adding new group `qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq' (1005) ...
groupadd: 'qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq' is not a valid group name
adduser: `/usr/sbin/groupadd -g 1005 qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq' returned error code 3. Exiting.

 
So it seems fine to me that busybox's adduser and addgroup mimic this behaviour
by checking for illegal characters and check the length of user names at creation
time and not after the accounts have been created. 
The values for the actual limits to enforce must not be necessarily the ones that
debian uses. Using LOGIN_NAME_MAX if defined or defining it ourselves
if not defined or using another value (32, 64, etc) is fine for me.
Also the exact regex for user names could be more or less
restrictive as deemed fit. We could even consider to make
it somehow configurable in menuconfig and supply some sane defaults that
could be changed and eventually turned off if desired.
The quality of code in the die_if_bad_username function could also be improved
to avoid the problems pointed out by Matthias Andree (missing casts to (unsigned char))
and jumping with goto into the loop (ask Denys about this).
 

Ciao,
Tito