6
\$\begingroup\$

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', '[email protected]');
$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 '<div style="background: #FF0000; color: #FFF; font-family: arial; font-size: 14px; padding: 20px; position: absolute; z-index:15; top:50%; left:50%; margin:-50px 0 0 -200px;">Access Denied. Wrong Username or Password. <a style="color: #FFF; text-decoration: none; background: #333; padding: 5px;" href="index.php">Go Back</a></div>';
                        return false;
                    }
                }
            }
        }
\$\endgroup\$

1 Answer 1

6
\$\begingroup\$

This is absolutely insecure.

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

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

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

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

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

\$\endgroup\$
2
  • \$\begingroup\$ Thanks for replying me. $password = $_POST['pwd']; $hashAndSalt = password_hash($password, PASSWORD_BCRYPT); and to access if(password_verify($password, $row['user_pwd']) && $row['user_name'] == $username) . Is this codes strong enough to protect attacks? and what about XSS attacks? \$\endgroup\$ Aug 17, 2014 at 9:50
  • 2
    \$\begingroup\$ I really suggest you first understand what an "attack" is, and what can and cannot be "attacked". Learning how an attack is done can be very useful. In this code, you have no risk of XSS. I also suggest you understand what Cross site scripting is. These two things are covered in depth on OWASPs information site. \$\endgroup\$
    – Alex L
    Aug 17, 2014 at 15:43

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.