1

I am using preg_match_all in php to check the characters in the username and password before i add them to the database, but I can't seem to get it to work the way I want. Here is what I have right now:

preg_match_all(USERNAME_PATTERN,$username,$usernameMatches);
preg_match_all(PASSWORD_PATTERN,$password,$passwordMatches);

Here are the patterns, defined as constants:

/*Username and Password Patterns*/
define("USERNAME_PATTERN","[-*_a-z0-9A-Z.]");
define("PASSWORD_PATTERN","[_a-z0-9A-Z]");

I don't know what is wrong with it. Its suppose to check to see if the username has anything other than a-z, A-Z, 0-9, the dash, the astrisk,the underscore, and a period. The password is the same as the username.

Here is the code I use to check:

if ($usernameMatches == 0){
echo("Bad characters in username<br />");
}

The password is the same.

knittl
  • 246,190
  • 53
  • 318
  • 364
legobear154
  • 431
  • 2
  • 6
  • 16

4 Answers4

3

There are several issues with your code.

  1. Your regexes only match a single character.
  2. There are no begin and end anchors in your regex.
  3. Make sure you instantiate the matches array before calling preg_match_all()
  4. Your regex should be surrounded with a / (or other valid char).
  5. Check for a non-match by checking that the array is empty, not by checking if it's equal to zero. There are many type/value checking gotchas in php and it's best to avoid them.

Try this:

/*Username and Password Patterns*/
define("USERNAME_PATTERN","/^[-*_a-z0-9A-Z.]+$/");
define("PASSWORD_PATTERN","/^[_a-z0-9A-Z]+$/");

$usernameMatches = array();
$passwordMatches = array();

preg_match_all(USERNAME_PATTERN,$username,$usernameMatches);
preg_match_all(PASSWORD_PATTERN,$password,$passwordMatches);

if (empty($usernameMatches)){
    echo("Bad characters in username<br />");
}

if (empty($passwordMatches)){
    echo("Bad characters in password<br />");
}

BTW: Your code could simplified by simply using preg_match() instead of preg_match_all(). Something like this should work as well as your code:

/*Username and Password Patterns*/
define("USERNAME_PATTERN","/^[-*_a-z0-9A-Z.]+$/");
define("PASSWORD_PATTERN","/^[_a-z0-9A-Z]+$/");

if (!preg_match(USERNAME_PATTERN, $username)) {
    echo("Bad characters in username<br />");
}
if (!preg_match(PASSWORD_PATTERN, $password)) {
    echo("Bad characters in password<br />");
}
Asaph
  • 159,146
  • 25
  • 197
  • 199
  • Can't test right now, but don't you also need to escape ".", "*" and "-"? – Joachim Isaksson Jan 28 '12 at 15:19
  • 1
    @Joachim Isaksson: No. Dot and star are _not_ meta chars when they are _inside a character class_. And the minus sign is not the range meta character in a character class if it is in the first or last position. – Asaph Jan 28 '12 at 15:26
  • Thank you, I have changed to preg_match and am using your way as it is indeed easier to do. – legobear154 Jan 28 '12 at 15:33
  • @legobear154: You're welcome. Please mark my answer correct by clicking the checkmark to the left of the answer. – Asaph Jan 28 '12 at 16:01
  • @Asaph I'm not sure which of our answers is better, but in any case I have voted for yours. You have certainly deserved more than 0 votes for that effort. 0.o – robert Jan 28 '12 at 19:52
2

Use this:

define("USERNAME_PATTERN","/^[-*_a-z0-9A-Z.]+$/");
define("PASSWORD_PATTERN","/^[_a-z0-9A-Z]+$/");

Currently you allow for single-character usernames and passwords only. Also you have forgotten to encapsulate the regular expressions by / (or another character). (This is specific to PHP and some other languages, admittedly.) I have also added ^ and $ so that the whole input string is matched.

By the way, why bother with checking the passwords? Just require a certain minimum length, for example (but not at all checked for security):

define("PASSWORD_PATTERN","/^.{6,}$/");

Also, I don't get why you're using preg_match_all. A preg_match should do as well, and is probably easier to use:

if (!preg_match(USERNAME_PATTERN, $username) {
    echo("Bad characters in username<br />");
}
robert
  • 3,484
  • 3
  • 29
  • 38
  • Ok, thank you. I will probably just limit the length of the password only. – legobear154 Jan 28 '12 at 15:16
  • Why limit the length? Hopefully you're not storing the passwords in cleartext; you should calculate a hash of the passwords that you store in your database. And even that is considered insecure nowadays; salted hashs are the way to go. – robert Jan 28 '12 at 15:19
  • I am encrypting the password. I am salting it as well. I honestly don't know what was going through my head when I first did that. Thank you all for your help! Could anyone point me to a good site for learning how to create patterns primarily for PHP? – legobear154 Jan 28 '12 at 15:25
1

don't know what is wrong with it. Its suppose to check to see if the username has anything other than a-z, A-Z, 0-9, the dash, the astrisk,the underscore, and a period. The password is the same as the username.

You should check, if passed username/password is valid. You need this pattern. /^[\-\*\w\d\.]{6,12}$/, Here minimum and maximum length is 6 and 12 respectively.

define('PATTERN', '/^[\-\*\w\d\.]+$/');
if(preg_match(PATTERN, $username)){
// username is correct
} else {
// username is wrong.
}

Same for Password.

Shiplu Mokaddim
  • 56,364
  • 17
  • 141
  • 187
1

Before answering I would just say that by asking that question, I suspect that you are saving passwords directly as clear text. This is not a good solution because it exposes your users passwords. There is a discussion around that here with examples on how to do this in a more secure way.

As an added bonus the problem with invalid characters (in the password) will not be an issue for most cases, unless you are dealing with some legacy systems. The reason for this is that you will not store the actual password, but just the generated hash

Back to your question. One alternative I like is to check for the precense of any invalid characters. By adding ^ to the character list, you will match any character other than the list.

define("USERNAME_PATTERN","/[^*_a-z0-9A-Z.-]/");
if(preg_match(USERNAME_PATTERN, $username))
    echo 'Bad characters in username';
Community
  • 1
  • 1