2

I'm trying to learn the fundamentals of PDO. I've built the following that inserts data into my table but I would like to get feedback on whether or not this is secure or whether it could have been done better?

Would my post variables need to be escaped like you would with mysql_real_escape_string()?

$firstname = $_POST['First_Name'];
$surname = $_POST['Surname'];
$nicknames = $_POST['Nicknames'];
$age = $_POST['Age'];


// Connection data (server_address, database, name, poassword)
$hostdb = 'localhost';
$namedb = 'tsite_co_uk';
$userdb = 'access@site.co.uk';
$passdb = 'password';

try {
  // Connect and create the PDO object
  $conn = new PDO("mysql:host=$hostdb; dbname=$namedb", $userdb, $passdb);
  $conn->exec("SET CHARACTER SET utf8");      // Sets encoding UTF-8
  $conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

  // Define an insert query
  $sql = "INSERT INTO `directory` 

  (`First_Name`,`Surname`,`Nicknames`,`Age`) 

  VALUES ('$firstname','$surname','$nicknames','$age')

  ";

  $count = $conn->exec($sql);

  $conn = null;        // Disconnect
}
catch(PDOException $e) {
  echo $e->getMessage();
}
Saurav Rastogi
  • 9,575
  • 3
  • 29
  • 41
Liam
  • 9,725
  • 39
  • 111
  • 209
  • A better place to ask this type of question is http://codereview.stackexchange.com/. That being said, NO...not secure. Learn how to use prepared statements: http://php.net/manual/en/pdo.prepared-statements.php – Jeremy Harris Nov 13 '12 at 13:49

1 Answers1

1

This is not secure--you're doing nothing to prevent SQL injection from the user supplied $_POST values. You should use prepared statements and bind the vales to them:

$conn = new PDO("mysql:host=$hostdb; dbname=$namedb", $userdb, $passdb);
$conn->exec("SET CHARACTER SET utf8");      // Sets encoding UTF-8
$conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

 $sql = "INSERT INTO `directory`  (`First_Name`,`Surname`,`Nicknames`,`Age`) 
     VALUES (:firstname ,:surname,:nicknames ,:age) ";

 $statement = $conn->prepare($sql);
 $statement->bindValue(":firstname", $firstname);
 $statement->bindValue(":surename", $surename);
 $statement->bindValue(":nicknames", $nicknames);
 $statement->bindValue(":age", $age);

 $count = $statement->execute();
Ray
  • 40,256
  • 21
  • 101
  • 138
  • Thanks @ray, would I still need to define $firstname etc variables using my post values firstly? $firstname = $_POST['First_Name']; $surname = $_POST['Surname']; $nicknames = $_POST['Nicknames']; $age = $_POST['Age']; – Liam Nov 13 '12 at 14:11
  • 1
    @Liam yes, I'm assuming everything before the creation of the connection is being done. Also, this is all inside the try/catch block. – Ray Nov 13 '12 at 14:14