0

I have a function which strips chars from my passwords.

public function stripChars($var)
{
    $bad_symbols = array(",", "|", "'","<",">",'"');
    $var = str_replace($bad_symbols, "", $var);
    return $var;
}

I'm wondering if there is a way to let the user's use a comma in their passwords, without it messing up the SQL statement? If not, which symbols should I not allow in passwords besides the ones I'm already stripping. Thanks.

user2128903
  • 57
  • 1
  • 7
  • 9
    None. Let them use whatever characters they want. – John Conde Mar 15 '13 at 21:12
  • 1
    The fact that you said `without it messing up the SQL statement` means you're probably not cleaning your data correctly. – Axel Mar 15 '13 at 21:13
  • 4
    You don't change the user's password. You modify your code to safely accept ANYTHING in/through/past your DB-handling code. e.g. use a PDO prepared statement. Consider a "knowledgeable" user whose password is `<,>|<">`. Your wonderful "security" system has now made their password blank, allowing anyone into their account. – Marc B Mar 15 '13 at 21:13
  • 2
    Additionally, you should not be storing the password in the database directly. You should be hashing it first (never store plain text passwords). – Wilduck Mar 15 '13 at 21:13
  • Oops, sorry I forgot I will be hashing it. – user2128903 Mar 15 '13 at 21:15
  • @user2128903 if you are hashing it why do these characters matter? It will hash regardless if they are there or not. Removing them will just end up changing the hash so it won't match up with the password the user enters and break your login system. – kittycat Mar 15 '13 at 21:18
  • Yes, I know. I had a programming goof. I'm paranoid! And over thinking the problem! :p – user2128903 Mar 15 '13 at 21:19

4 Answers4

9

Which characters should I strip from the user's password, before inserting into database?

None at all. The more you characters you remove, the weaker you make the password.

I'm wondering if there is a way to let the user's use a comma in their passwords, without it messing up the SQL statement?

  1. Don't build SQL by hand. Use prepared statements and bound variables. That will escape any characters that might cause problems.
  2. Don't store plain text passwords in your database. Hash them.
Community
  • 1
  • 1
Quentin
  • 914,110
  • 126
  • 1,211
  • 1,335
  • Do you guys use strip_tags for usernames and such? – user2128903 Mar 15 '13 at 21:22
  • No. `strip_tags` is a brute force and poor quality XSS filter. I'll either just escape the characters or be specific (with a whitelist) about what is allowed into the username in the first place. – Quentin Mar 15 '13 at 21:30
1
without it messing up the SQL statement?

Ouch, you need to read up on SQL injection: http://en.wikipedia.org/wiki/SQL_injection

So you should allow any character in the password and make sure you do not use the input directly in the SQL statement.

Pleun
  • 8,856
  • 2
  • 30
  • 50
1

You should never strip characters from a user's password. If they enter their password as "r2^£XS\'32" and then you filter it before storing it, they'll never be able to log in as their password will be incorrect.

As for whether you should restrict characters in a password, that's a different matter. You'll need to restrict characters that are not part of whatever character coding you use, but aside from that it's a bad idea. Limiting the character set available makes it easier for brute force attacks to be successful.

Instead, what you want to do is use prepared statements as these will prevent SQL injection (which is, I believe, your concern).

It's also worth mentioning that you should hash your passwords before storing them; never store as plain-text or as reversible encryptions. Ideally, also salt the password for additional security.

Check out Password Hashing and PDO and Prepared Statements for more information

Michael
  • 11,912
  • 6
  • 49
  • 64
-1

You shouldn't remove any characters from passwords! However, do 'escape' them before putting them into the database or comparing for authentication.

mavili
  • 3,385
  • 4
  • 30
  • 46
  • What for? Are you going to display the password somewhere? – Mchl Mar 15 '13 at 21:15
  • The only time you should use `htmlspecialchars` is when you are inserting data into an HTML document (not a database!). You should never put users' passwords into an HTML document. – Quentin Mar 15 '13 at 21:16
  • I haven't said you should display it or something, but you're right there's no point in using htmlspecialchars with passwords and in fact it might even be wrong as it changes < to < ... etc. removed that from the answer – mavili Mar 15 '13 at 21:19
  • mavili, yes I have been escaping them. – user2128903 Mar 15 '13 at 21:23
  • 1
    Then you don't need anything else. Except that keep in mind that you shouldn't store passwords in plain text in the DB. Hash them. – mavili Mar 15 '13 at 21:28