HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Secure way to store passwords

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
passwordssecurewaystore

Problem

Here I'm trying to develop a simple framework for my own purpose. I'm using PHP and PDO. But, I'm still worried about my password security codes. Below I have posted my codes. it stored like these in my database $1$sr1.F90.$hXd.9DheSMtYmI52KT7fy0. Is this the right way to store passwords? Or Am I make anything wrong here?

I am worried about SQL injection and XSS attacks. But, here I'm using PDO. Is this code good enough to prevent attacks?

Insert script

$password = $_POST['pwd'];
$crypted_pass = crypt($password);
date_default_timezone_set('Asia/Kolkata');
$date = date('Y-m-d h:i:s');
$insert_query = $dbh->prepare("INSERT INTO admin ( user_name, user_pwd, user_email, profile_created_on ) VALUES ( :username, :userpwd, :useremail, :date )");
$insert_query->bindValue(':username', 'Administrator');
$insert_query->bindValue(':userpwd', $crypted_pass);
$insert_query->bindValue(':useremail', 'something@gmail.com');
$insert_query->bindParam(':date', $date);
$insert_query->execute();


Access script

```
if(isset($_POST['admin_login']))
{
$username = mysql_real_escape_string($_POST['admin_user_name']);
$password = mysql_real_escape_string($_POST['admin_user_pwd']);
try
{
$select_query = $dbh->prepare("SELECT * FROM admin WHERE user_name = '$username'");
$result = $select_query->execute();
if(count($result) > 0)
{
while($row = $select_query->fetch(PDO::FETCH_ASSOC))
{
if(crypt($password, $row['user_pwd']) == $row['user_pwd'] && $row['user_name'] == $username)
{
session_start();
$_SESSION['sessionname'] = $row['user_name'];
header('Location: dashboard.php');
return true;
}
else
{
echo 'Access Denied. Wrong Username or Password. Go Back';

Solution

This is absolutely insecure.

I will list only a handful of things which I think need to be absolutely rewritten.

-
Do not use crypt. Instead, please use password_hash. Along with password_verify, your code could be simplified a great deal.

-
If you're going to use PDO, use it. Do not mix in other database access modules (MySQL). Especially not MySQL, it is not current and is not supported nor will it ever be supported in the future. mysql_real_escape_string gives a false sense of protection, but in reality, it does nothing for you.

-
Along with number 2, comes the possibility of SQL injection in your app. If you follow what I've said in number 2 (Use PDO (bind parameters, prepare statements), get rid of anything MySQL), you should be good.

-
If you're going to try something, catch it! I want to know when something goes wrong.

Context

StackExchange Code Review Q#60238, answer score: 6

Revisions (0)

No revisions yet.