Hi, this patch improves the checks performed on usernames with the die_if_bad_username() function by adduser and addgroup. The changes are: 1) better comments; 2) use of the portable filename character set plus '@' and '$'; 3) don't use isalnum as it will allow non-ASCII letters in legacy 8-bit locales as pointed out by Rich Felker; 3) enforce minimum length of 1 char (or at least 2 chars if '$' is used as last char); 4) enforce maximum length of LOGIN_NAME_MAX (including null termination); 5) don't use goto to jump into the loop (requested by Matthias Andree); 6) don't allow '$', '.', '@' and '-' as first char; 7) allow '$' only as last char. 8) don't print the illegal char in error message as if it is a wide char it will be unreadable.
On Debian the default is more conservative as defined by the regular expression /^[a-z][-a-z0-9]*$/ we use a more permissive /^[_A-Za-z0-9][-\@_.A-Za-z0-9]*\$?$/ instead. The function is tested with this test cases: "", "$", "a", "aa", "aA", "@a", "a@", "a@a", ".a", "a.", "a.a", "-a", "a-", "a$", "$a", "a$a", "_a", "a_", "a_a", "a1", "1a", "a?", "aè", "a€", "aQzECE2-A_l@s$" and seems to behave well. I would like to point out that this check can be simply turned off by disabling FEATURE_CHECK_NAMES in config (Enable sanity check on user/group names in adduser and addgroup) to avoid mixing of "mechanism (code) and policy" and also that it mimics what adduser/addgroup do on my debian box (with a more restrictive policy). Hints, critics and improvements are welcome. Special thanks go to Matthias Andree and Rich Felker for the patience thay had in explaining me my errors. Please apply if you like it. Ciao, Tito /* The username to "(...) be portable across systems conforming to IEEE Std * 1003.1-2001, (...) is composed of characters from the portable * filename character set. The hyphen should not be used as the first character * of a portable username" (SUSv3). The Portable Filename Character Set is: * 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 . _ - * We allow also the '@' and the '$' sign, but to avoid problems, '@','$' and '.' * are not permitted at the beginning of the username (for example, a name * like ".." can make adduser chown "/home/..). * For compatibility with Samba machine accounts '$' is supported * (only) at the end of the username. On Debian the default is more conservative as * defined by the regular expression /^[a-z][-a-z0-9]*$/. */ void FAST_FUNC die_if_bad_username(const char *name) { const char *s = name; assert(name != NULL); /* The minimum size of the login name is one char or two if * last char is the '$', this exception is catched later * as the dollar sign could not be the first char. * The maximum size of the login name is LOGIN_NAME_MAX * including the terminating null byte. */ if (!*name || strlen(name) + 1 > LOGIN_NAME_MAX) bb_error_msg_and_die("illegal name length"); do { /* We don't use isalnum as it will allow locale-specific non-ASCII */ /* letters in legacy 8-bit locales. */ if (((*name == '-' || *name == '.' || *name == '@') && name != s) /* not as first char */ || (*name == '$' && (!name[1] && name != s)) /* not as first, only as last char */ || *name == '_' || (*name >= 48 && *name <= 57) /* 0-9 */ || (*name >= 65 && *name <= 90) /* A-Z */ || (*name >= 97 && *name <= 122) /* a-z */ ) { continue; } bb_error_msg_and_die("illegal character in name"); } while (*++name); }
--- libbb/die_if_bad_username.c.orig 2011-02-03 21:30:50.000000000 +0100 +++ libbb/die_if_bad_username.c 2011-08-01 21:27:50.000000000 +0200 @@ -8,33 +8,51 @@ */ #include "libbb.h" +#include <assert.h> -/* 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. +/* The username to "(...) be portable across systems conforming to IEEE Std + * 1003.1-2001, (...) is composed of characters from the portable + * filename character set. The hyphen should not be used as the first character + * of a portable username" (SUSv3). The Portable Filename Character Set is: + * 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 . _ - + * We allow also the '@' and the '$' sign, but to avoid problems, '@','$' and '.' + * are not permitted at the beginning of the username (for example, a name + * like ".." can make adduser chown "/home/..). + * For compatibility with Samba machine accounts '$' is supported + * (only) at the end of the username. On Debian the default is more conservative as + * defined by the regular expression /^[a-z][-a-z0-9]*$/. */ void FAST_FUNC die_if_bad_username(const char *name) { - /* 1st char being dash or dot isn't valid: */ - goto skip; - /* For example, name like ".." can make adduser - * chown "/home/.." recursively - NOT GOOD - */ + const char *s = name; + assert(name != NULL); + + /* The minimum size of the login name is one char or two if + * last char is the '$', this exception is catched later + * as the dollar sign could not be the first char. + * The maximum size of the login name is LOGIN_NAME_MAX + * including the terminating null byte. + */ + if (!*name || strlen(name) + 1 > LOGIN_NAME_MAX) + bb_error_msg_and_die("illegal name length"); + do { - if (*name == '-' || *name == '.') - continue; - skip: - if (isalnum(*name) + /* We don't use isalnum as it will allow locale-specific non-ASCII */ + /* letters in legacy 8-bit locales. */ + if (((*name == '-' || *name == '.' || *name == '@') && name != s) /* not as first char */ + || (*name == '$' && (!name[1] && name != s)) /* not as first, only as last char */ || *name == '_' - || *name == '@' - || (*name == '$' && !name[1]) + || (*name >= 48 && *name <= 57) /* 0-9 */ + || (*name >= 65 && *name <= 90) /* A-Z */ + || (*name >= 97 && *name <= 122) /* a-z */ ) { continue; } - bb_error_msg_and_die("illegal character '%c'", *name); + bb_error_msg_and_die("illegal character in name"); } while (*++name); } +
_______________________________________________ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox