login security

[vertex78]

Can you guys tell me if you see any major security flaws in this login method, the $user and $password variables are taken straight from html form.

 

public function login($user,$password){
	$encrypted_password = crypt($password,"afYHbf765Hmd3asdft1mpoz");
	if(1 == mysql_num_rows($result = mysql_query("SELECT * from users WHERE userName='$user' and password = '$encrypted_password'"))){
		$_SESSION['user']=$user;
		$_SESSION['logged_in']=true;

		//get privelege level
		$result = mysql_query("SELECT privilegeLevel from users WHERE userName='$user' and password = '$encrypted_password'");
		$row = mysql_fetch_array($result);
		$_SESSION['user_privilege_level'] = $row[0];
		return true;
	}
	else {
    		$_SESSION['logged_in']=false;
    		if (empty($result)){
        		echo mysql_error();
    		}
		return false;
	}

}

[rhodesa]

First, when using crypt, you should use a random salt. If you don't supply a second argument, PHP will supply a random one for you. Then, use their encrypted password as the salt when checking it. Like this:

 

public function login($user,$password){
	//First select the user record
	$result = mysql_query("SELECT * from users WHERE userName='$user'");
	if($result && mysql_num_rows($result)){
		//User found
		$user = mysql_fetch_assoc($result);
		$encrypted_password = crypt($password,$user['password']);
		//Check password
		if(!strcmp($encrypted_password,$user['password'])){
			//Password is good
			$_SESSION['user']=$user;
			$_SESSION['logged_in']=true;
			$_SESSION['user_privilege_level'] = $user['privilegeLevel'];
			return true;
		}
	}
	//Don't need an else. If it gets here, something went wrong
	$_SESSION = array('logged_in' => false); //This will clear all session variables
    		if (empty($result)) {
        		echo mysql_error();
	}
	return false;
}