Reputation: 1685
I've the following code:
$salt=uniqid(mt_rand(), false);
#Add data to tables
mysql_query("INSERT INTO accounts VALUES('$user', '".hash('sha512',$pass+$salt)."', '$salt', '$cookie_value')");
mysql_query("INSERT INTO passwordreset VALUES('$user', NULL, NULL)");
#cookie creation
#.....
#cookie update
mysql_query("UPDATE accounts SET cookie='$cookie_value' WHERE user='$user'");
I sanitize data from form using these functions:
$var = htmlentities($var, ENT_QUOTES, "UTF-8");
return mysql_real_escape_string($var);
Today I logged into phpMyAdmin and I saw that passwords and salts for all users are the same. Don't remind me about deprecated mysql_* I know it, that's just quick draft.
Upvotes: 1
Views: 103
Reputation: 173522
There's a few things I would comment on your current code:
Using the +
operator (as opposed to .
) on two strings results in the sum of both values cast to integer (if a string is not numeric it's cast to int(0)
); when it's passed to hash()
it gets cast to a string again, so your passwords will typically all be sha512("0")
. I'm not sure why your salts all have the same value though, unless the column data type is INT
in your database.
You can use uniqid(mt_rand(), true)
to collect more entropy, resulting in a better salt.
You should hash passwords with a dedicated password hash, such as the BlowFish option in crypt()
(make sure your column width is big enough); this way you can get rid of the salt column and you can choose how much work is required to verify the hash in a backward compatible manner.
The cookie column is for an auto-login feature I assume? It's better to create a separate table for this containing a randomized string as the primary key and a foreign key to your users table. This way you can support auto-login from multiple browsers.
Upvotes: 1
Reputation: 12830
error here
$pass+$salt
should be
$pass.$salt
. is used for string catenation in php
Upvotes: 0
Reputation: 7212
String concatenation in PHP uses .
not +
. Thus:
hash('sha512',$pass+$salt)
Should be
hash('sha512',$pass.$salt) // or
hash('sha512',"${pass}${salt}")
Upvotes: 3