Skip to content

is_valid_name(): What conforms a valid name? #836

@alejandro-colomar

Description

@alejandro-colomar

There's is_valid_user_name():

alx@debian:~/src/shadow/shadow/wextra$ grepc -ntfd is_valid_user_name src/ lib*


lib/chkname.c:75:
bool is_valid_user_name (const char *name)
{
	size_t  maxlen;

	/*
	 * User names length are limited by the kernel
	 */
	maxlen = sysconf(_SC_LOGIN_NAME_MAX);
	if (strlen(name) > maxlen)
		return false;

	return is_valid_name (name);
}

Which limits the length of a valid user name by sysconf(_SC_LOGIN_NAME_MAX). But it calls is_valid_name():

alx@debian:~/src/shadow/shadow/wextra$ grepc -ntfd is_valid_name src/ lib*


lib/chkname.c:28:
static bool is_valid_name (const char *name)
{
	if (allow_bad_names) {
		return true;
	}

	/*
         * User/group names must match gnu e-regex:
         *    [a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,30}[a-zA-Z0-9_.$-]?
         *
         * as a non-POSIX, extension, allow "$" as the last char for
         * sake of Samba 3.x "add machine script"
         *
         * Also do not allow fully numeric names or just "." or "..".
         */
	int numeric;

	if ('\0' == *name ||
	    ('.' == *name && (('.' == name[1] && '\0' == name[2]) ||
			      '\0' == name[1])) ||
	    !((*name >= 'a' && *name <= 'z') ||
	      (*name >= 'A' && *name <= 'Z') ||
	      (*name >= '0' && *name <= '9') ||
	      *name == '_' ||
	      *name == '.')) {
		return false;
	}

	numeric = isdigit(*name);

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

	return !numeric;
}

Which in a comment, says the name should match a regex that allows no more than 32 characters. I don't see the code actually enforcing that regex, as the loop seems unlimited. So, I have some questions:

  • Should we be limiting valid user names to 32 characters?
  • Or should we change that regex to specify what the code is really doing?

Or am I missing something?

Cc: @ikerexxe This was triggered by your suggestion of writing tests for this function. :)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions