On Monday 01 August 2011 22:01, Tito wrote:
> 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;
We use ASCII-only isalnum throughout bbox anyway. But ok.
> 3) enforce minimum length of 1 char (or at least 2 chars if '$' is used as
> last char);
This was already the case, I think.
> 4) enforce maximum length of LOGIN_NAME_MAX (including null termination);
Ok.
> 5) don't use goto to jump into the loop (requested by Matthias Andree);
Don't see why. It's efficient.
> 6) don't allow '$', '.', '@' and '-' as first char;
> 7) allow '$' only as last char.
Ok.
> 8) don't print the illegal char in error message as if it is a wide char it
> will be unreadable.
Ok.
On Tuesday 02 August 2011 21:57, Tito wrote:
> 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");
We can check length at the end by calcualting (name - s).
This way, we don't need strlen call.
Zero-length check is not necessary as this case is already caught
by the checking loop.
> 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 */
Come to think about it, '@' can't be used in usernames:
@ is used to delimit username from hostname:
user@host
> || *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);
> }
How about this?
void FAST_FUNC die_if_bad_username(const char *name)
{
const char *start = name;
/* 1st char being dash or dot isn't valid:
* for example, name like ".." can make adduser
* chown "/home/.." recursively - NOT GOOD.
* Name of just a single "$" is also rejected.
*/
goto skip;
do {
unsigned char ch;
/* These chars are valid unless they are at the 1st pos: */
if (*name == '-'
|| *name == '.'
/* $ is allowed if it's the last char: */
|| (*name == '$' && !name[1])
) {
continue;
}
skip:
ch = *name;
if (ch == '_'
/* || ch == '@' -- we disallow this too. Think about
"user@host" */
/* open-coded isalnum: */
|| (ch >= '0' && ch <= '9')
|| ((ch|0x20) >= 'a' && (ch|0x20) <= 'z')
) {
continue;
}
bb_error_msg_and_die("illegal character with code %u at
position %u",
(unsigned)ch, (unsigned)(name - start));
} while (*++name);
/* The minimum size of the login name is one char or two if
* last char is the '$'. Violations of this are caught above.
* The maximum size of the login name is LOGIN_NAME_MAX
* including the terminating null byte.
*/
if (name - start >= LOGIN_NAME_MAX)
bb_error_msg_and_die("name is too long");
}
function old new delta
die_if_bad_username 77 97 +20
--
vda
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
