treng
treng

Reputation: 1685

Error after INSERT INTO

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

Answers (4)

Ja͢ck
Ja͢ck

Reputation: 173522

There's a few things I would comment on your current code:

  1. 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.

  2. You can use uniqid(mt_rand(), true) to collect more entropy, resulting in a better salt.

  3. 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.

  4. 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

amitchhajer
amitchhajer

Reputation: 12830

error here

$pass+$salt

should be

$pass.$salt

. is used for string catenation in php

Upvotes: 0

xdazz
xdazz

Reputation: 160833

This is PHP, $pass+$salt should be $pass . $salt

Upvotes: 2

Kurt
Kurt

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

Related Questions