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

Reply via email to