PDO Insert Snippet

[joshuasa9]

Hey all, 

 

I have a quick question regarding the PDO object when querying the MySQL database.

 

I have created a simple form to create a user with a username and password for now.  I wrote this snippet based on some research I have done and just want to check with some more experienced users if this will pass.

 

Here it is:

 

function CreateAdmin($post)
{
    $pdo = GetConnection();
    
    $sql = "INSERT INTO admin_users (username, password) VALUES (:username, :password)";
    
    $username = $post['username'];
    $password = FormatPassword($post['password']);
    
    $statement = $pdo->prepare($sql);
    
    $statement->execute(array(':username'=>$username, ':password'=>$password));
}

 

 

 

Now the FormatPassword function you see is a function I created that will encrypt the password givin with a mixture of 2 types of methods in PHP - MD5 and sha1.  Here is that function:

 

function FormatPassword($pass)
{
    $salt = "@x2p";
    $hash = sha1(md5($salt.$pass)).md5($pass).sha1(md5(md5($pass)));
    
    return $hash;
}

 

 

 

 

I appreciate any input I can get!

 

Thanks!

[matthew.javelet]

I'm not sure exactly what you're asking, but your code will work. It just has absolutely no error management which can lead to a compromised database.

 

I don't know how the rest of your code is used so I'm not going to say the create user function isn't needed.  I would just generate a random salt instead of using @2xp just to be extra safe.

[joshuasa9]

Thanks for you reply!

 

I'm just beginning to use PHP for a project I'm doing and was researching ways to communicate with MySQL.  I found the MSQLi and PDO styles the best ways.  Which do you prefer and why?

[matthew.javelet]

I use MySQL Improved ( MySQLi ). I've heard a bunch of hype about PDO because of its wondrous prepared statements and nifty functions. But I think it's pretty much a pre-built database class, though I may be wrong.

 

By MSQLi do you mean MySQLi or MiniSQL? Either way, PDO is just a class, it is not a storage system like Oracle, MSQL, PostgreSQL, etc.

Also, I'd prefer MySQLi over basic MySQL for obvious reasons.

 

Source:

http://www.php.net/manual/en/intro.pdo.php

Edited March 5, 2013 by matthew.javelet

[salathe]

I have a couple of comments, though they're not particularly anything to do with PDO at all. That side of things looks okay in your code.

 

FormatPassword()

 

This is a crazy way to hash a password, and going the extra mile to call sha1()/md5() six times is not doing you any favours. Instead, look towards using a different hashing implementation. For example, using the crypt() function with Blowfish and a suitable number of "rounds" would be an infinitely better choice. It's probably beyond the scope of a quick reply to delve into why this is so, some quick searching on the subject will bring up a wealth of information if you're interested. Also, if you're brave enough to be trying out the PHP 5.5.0 alphas then you could play with the password_hash() function which looks to introduce a new best-practice function for dealing with passwords in PHP.

 

Since you're probably not able to use PHP 5.5.0 yet then here's what a call to crypt() might look like.

$salt = ...; // Should be 22 characters of "./0-9A-Za-z" for Blowfish
$hash = crypt($pass, '$2y$08$'.$salt);

Special Note: Please carefully read the notes about the $2a$, $2x$, and $2y$ salt prefixes, in particular which versions of PHP have them. Also, be sure to set the number of rounds (08) to something sensible for your needs. A lower number makes the hashing consume less time, which is good for anyone trying to attack your system. A higher number makes hashing consume more time, which if you go crazy can cause creating a hash to take a very, very long time. Have a play around with the rounds, to see what works best for you.

 

Salting

 

Your code has a single salt for all passwords. This is also not a very good idea. It is much better practice to have a different salt for each password. This can be stored in the database too, to be looked up when checking the password later. For why this is a good idea, have a read up on salts and their effect on using "rainbow tables".

Edited March 5, 2013 by salathe