Hi,
minor improvements vs. v2 patch:
1) some more comments added.
2) we now print the position of the illegal character.
Hints, critics, improvements are welcome.
Thanks to all who helped.
Ciao,
Tito
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 at position '%d'", name
- s);
} while (*++name);
}
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
print its position instead.
Signed-off-by: Tito Ragusa <[email protected]>
--- libbb/die_if_bad_username.c.orig 2011-02-03 21:30:50.000000000 +0100
+++ libbb/die_if_bad_username.c 2011-08-02 21:43:13.000000000 +0200
@@ -8,33 +8,53 @@
*/
#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. Be aware that it might be necessary
+ * to quote or escape the '$' sign to avoid that the shell mangles it.
+ * 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 at position '%d'", name - s);
} while (*++name);
}
+
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
