There is a serious bug in buddypress 1.2.3 signup validation for username

[faisalzulfiqar]

Hi I work at Mindblaze Technologies,

I was deploying a site for one of our clients opentuition.com and I found this.

In bp-core/bp-core-signup.php

This code has serious problem:

$db_illegal_names = get_site_option( 'illegal_names' );
$filtered_illegal_names = apply_filters( 'bp_core_illegal_usernames', array( 'www', 'web', 'root', 'admin', 'main', 'invite', 'administrator', BP_GROUPS_SLUG, BP_MEMBERS_SLUG, BP_FORUMS_SLUG, BP_BLOGS_SLUG, BP_REGISTER_SLUG, BP_ACTIVATION_SLUG ) );

$illegal_names = array_merge( (array)$db_illegal_names, (array)$filtered_illegal_names );

in it "array_merge" function is embedding "array( 'www', 'web', 'root', 'admin', 'main', 'invite', 'administrator', BP_GROUPS_SLUG, BP_MEMBERS_SLUG, BP_FORUMS_SLUG, BP_BLOGS_SLUG, BP_REGISTER_SLUG, BP_ACTIVATION_SLUG )" at the end of "$db_illegal_names" so every time validation function is called it gets appended and the size of this field starts to increase until the point that it breaks the update query which becomes huge after a hundred sign ups or so.

the last line should be like this:
$common_names = array_intersect( (array)$db_illegal_names, (array)$filtered_illegal_names );
$diff_names = array_diff( (array)$db_illegal_names, (array)$filtered_illegal_names );
$illegal_names = array_merge( (array)$diff_names, (array)$common_names );

"array_merge" function merges arrays with numeric keys that is why here we can not use "array_merge"

[johnjamesjacoby]

(In [2928]) Fixes #2310 props faisalzulfiqar

[aesqe]

shouldn't that be:

$common_names = array_intersect( (array)$filtered_illegal_names, (array)$db_illegal_names );
$diff_names = array_diff( (array)$filtered_illegal_names, (array)$db_illegal_names );

$filtered_illegal_names should be first argument.

[johnjamesjacoby]

Seems to be working fine but will review for 1.2.5.

[cnorris23]

@aesqe

The two lines you reopened the ticket over are correct in the code. The $common_names and $diff_names variables are being constructed to update the database, and as such, the $db_illegal_names variable should be the first parameter of both array_intersect and array_diff.

[aesqe]

actually, it seems that this whole system is wrong:

$one = array(1,2,3,4,7);
$two = array(2,4,5,8,9);

$intersect = array_intersect($one, $two);
$diff = array_diff($one, $two);

$result = array_merge($diff, $intersect); Array ( [0] => 1 [1] => 3 [2] => 7 [3] => 2 [4] => 4 )

in the end, you get the first array again, and the filter does not work.

i thought the solution was to switch array positions because then, the filter worked.

i might be just mad tired, tho, so i'll review this again in the morning, but i think i'm right.

[cnorris23]

@aesqe
You're right. I was looking strictly at syntax. Going through and actually testing the code shows that things aren't working correctly. I played around a little, but it looks like the best solution is to go back to the code pre-[2928], and then run $illegal_names through array_unique(). However, I feel like I'm missing something, because this seems like an easier, and more obvious solution. Regardless, I've added a patch to add array_unique(). Can you test this?

[aesqe]

that patch works fine, but can i suggest something else?

// names already in db
$db_illegal_names = get_site_option('illegal_names');

// core values that mustn't be allowed to be registered as usernames:
$core_illegal_names = array( 'www', 'web', 'root', 'admin', 'main', 'invite', 'administrator', BP_GROUPS_SLUG, BP_MEMBERS_SLUG, BP_FORUMS_SLUG, BP_BLOGS_SLUG, BP_REGISTER_SLUG, BP_ACTIVATION_SLUG );

// so why supply users with $core_illegal_usernames via filter?
// aren't those names already saved to db when buddypress is activated?
// if they need to know those values, they can do get_site_option('illegal_names'), right?
// i think that this filter should be used just for adding names, se let's just give users an empty array:
$filtered_illegal_names = apply_filters("bp_core_illegal_usernames", array());

//and then merge all three:
$illegal_names = array_merge( (array)$db_illegal_names, (array)$filtered_illegal_names, (array)$core_illegal_names );
$illegal_names = array_unique( $illegal_names );

i hope i'm not missing something :)

[johnjamesjacoby]

(In [3037]) Fixes #2310