Am 01.08.2011 22:01, schrieb Tito:
> 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");
> 

        when there is a "char buf[LOGIN_NAME_MAX]", maybe you need
         strlen(name) + 1 >= LOGIN_NAME_MAX

                
>       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;
>               }


        the user should habe a change to understand what he did wrong ...
        perhaps you can do something like that:
        bb_error_msg_and_die("illegal character >%c< in name",*name);

>               bb_error_msg_and_die("illegal character in name");
>       } while (*++name);
> }

How important do you cansider that $ can be "nowhere else that last place" 
thing ?
So far i understand that Samba can life with $, dropping support for that would 
simplify the thing.
> 
> 
just my 2 cents,
re,
 wh


> 
> _______________________________________________
> busybox mailing list
> [email protected]
> http://lists.busybox.net/mailman/listinfo/busybox
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to