patternphpMinor
Custom login system (PDO, sanitizing, hash)
Viewed 0 times
systemloginhashcustomsanitizingpdo
Problem
I have made a custom login form for my site, with the help of PDO (until now I used simple MySQL connection and sanitizing), and it looks like this:
Is this safe? I know that nothing is 100% safe, but I have a small number of users (around 200-300), and 1000 visitors a month, so I don't expect very "professional" attacks and programming experts.
My questions are:
-
As you can see, I don't use sanitizing at all. I thought that PDO will do that part of the job, or I am wrong?
-
I hope that crypting of password is good (with no MD5 or SHA, is
-
File config.php contains username and pass of my database. Should I protect it somehow, what permissions to set?
-
Any other (serious) problem with this code?
session_name('NEWCOOKIE');
session_start();
require_once "config.php";
$member_username = $_POST['username'];
$member_password = $_POST['password'];
$crypt_pass = crypt($member_password,"somesalt");
try{
$dbh = new PDO('mysql:host=localhost;dbname='.DB_NAME, DB_USERNAME, DB_PASS, array(PDO::MYSQL_ATTR_INIT_COMMAND => "SET NAMES utf8"));
$dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
}
catch (PDOException $e) {
echo "Fatal error.";
file_put_contents('PDOErrors.txt', $e->getMessage(), FILE_APPEND);
}
$sth = $dbh->prepare("SELECT * FROM ".DB_PREFIX."_users WHERE username = :user AND password = :pass");
$sth->bindParam(':user', $member_username);
$sth->bindParam(':pass', $crypt_pass);
$sth->execute();
$total = $sth->rowCount();
$row = $sth->fetch();
if($total > 0){
if($row['activated']){
$_SESSION["user_username"] = $member_username;
$_SESSION["user_logedIn"] = true;
$_SESSION["user_id"] = $row['user_id'];
$_SESSION["timeout"] = time();
header("location: index.php");
}
else{
echo "ACCOUNT NOT ACTIVE";
}
}
else{
echo "WRONG PASSWORD OR USERNAME";
}Is this safe? I know that nothing is 100% safe, but I have a small number of users (around 200-300), and 1000 visitors a month, so I don't expect very "professional" attacks and programming experts.
My questions are:
-
As you can see, I don't use sanitizing at all. I thought that PDO will do that part of the job, or I am wrong?
-
I hope that crypting of password is good (with no MD5 or SHA, is
crypt() recommended)?-
File config.php contains username and pass of my database. Should I protect it somehow, what permissions to set?
-
Any other (serious) problem with this code?
Solution
Well, let's see:
- You're using PDO and prepared statements, no risk of SQL injection there. Great!
- Your PDO code may throw an Exception (Specifically, a
PDOException) at any time, so the whole database code block should be kept inside of thetry/catchblock.
cryptin on itself isn't 100% secure. See This question for more details.
- Database credentials are not constant. They can change over time, and you may want to change the server, change the user, add an additional database etc in the near/far future. The solution is to use functions (or better yet, classes) and pass the credentials as variables, rather then applying them as app-wide constants.
Context
StackExchange Code Review Q#14267, answer score: 5
Revisions (0)
No revisions yet.