sha1 + salt encryption fails in php

Question

I am trying to secure a password with sha1 hashing to a saltMe() function but the ectrypted password ends up the same no matter what I type in the password field. This might lead to a security vulnerability I think.

This is my salt function

**************************************************
* sha1 salt string to make a more secure hash 
***************************************************/
function SaltMe() {
 //Secret keyword encrypted with sha1
 return "4cefe49883b6dd1a00565e2a80fb035f348da3aa";
}

and this is my login check

$select_user_sql = $this->db->selectSQL("*", "tdic_users", "email = '". $email ."' AND password = '". sha1($this->main->SaltMe($password)) ."'");

No matter what I type in the password field I end up with:

1c2c2961d35148e8dfc83c7b31cf144f0987de9d

Which is also what my encrypted password is. But it is not good that I can type whatever I want to match the password.

The login form's actions is validatelogin.php which contain:

$user = new UserHandling();
$user->UserLogMeIn($_POST["login_email"], $_POST["login_password"]);

And the login function:

 /**********************************************************
 * User login function
 * 
 * @param string | User's email
 * @param string | User's password
 **********************************************************/
 function UserLogMeIn($email, $password) {
 $select_user_sql = $this->db->selectSQL("*", "tdic_users", "email = '". $email ."' AND password = '". sha1($this->main->SaltMe($password)) ."'");
 $select_user_result = $this->db->SQLquery($select_user_sql);
 if(mysql_num_rows($select_user_result) < 1) {
 $this->main->txtOutput("Wrong email or password", "TXT_ERR"); //The user typed something wrong.
 } else { 
 while($row = $this->db->SQLfetch($select_user_result)) {
 /*** We will check if user have activated the profile ***/
 if($row["activated"] == 0) {
 $this->main->txtOutput("Your profile haven't been activated! You need to click on the activation link in the email you recieved upon registration.", "TXT_ERR"); //The user haven't activated the new profile. This is necessary for security / spamming reasons
 $this->main->JSredirector("http://localhost/test/login.php", 5); //Redirect the user back from where he/she came from
 } else {
 /*** Everything is in order and we will let the user in ***/
 $_SESSION["usr_logged_in"] = 1;
 $_SESSION["user_email"] = $row["email"];
 $_SESSION["user_id"] = $row["user_id"];
 $_SESSION["user_name"] = $row["name"];
 /*** This will just update the last login field in the user table ***/
 $fields = array("user_last_logged_in" => time());
 $update_user_sql = $this->db->updateSQL('tdic_users', 'email = "'. $email .'"', $fields);
 $this->db->SQLquery($update_user_sql);
 }
 }
 }
 }

I can not figure out where the string is set so it always matches!

• SLaks: You must use a different salt for each password.
• SLaks: You have a SQL injection vulnerability.
• Joey: SHA-1 is not encryption, it's hashing.

Answer 1

Your SaltMe function doesn't get an argument, it just always returns the same string. So

SaltMe($password)

will not do what you intended.

In all seriousness, though: Stop trying to implement your own password hashing scheme. The very fact that you're here to ask about a mistake in your implementation should be proof enough that you don't understand enough to do so. Use a library, e.g. bcrypt (the Portable PHP Password Hashing Framework has an implementation) and stay far away from ever implementing any crypto code yourself.

Answer 2

U return a constant string when salting pass. Try

function SaltMe($pass) {
 // Pass salted with Secret keyword encrypted with sha1 
 return "4cefe49883b6dd1a00565e2a80fb035f348da3aa" . $pass;
}

Also as SLaks said, u have SQLinj. And it is better to use PDO or mysqli database functions.

Answer 3

You are only hashing the salt string, you haven't prefixed it to your password.

"4cefe49883b6dd1a00565e2a80fb035f348da3aa" -> [SHA-1] -> "1c2c2961d35148e8dfc83c7b31cf144f0987de9d"

You need to prefix your password with the salt before hashing it.