POST security with PDO?

Question

So I have some code that works as PDO. But is it secure? I'm trying to prevent SQL injection. This is my first PDO script and was hoping to see if it was done correctly. Here it is.

<?php
echo 'Your query code has been submited';

$host = "localhost";
$pass = "root";
$user= "root";
$dbname = "users";
try{
$con = new PDO("mysql:host=$host;dbname=$dbname", $user, $pass);
}
catch(PDOException $e){
    echo $e->getMessage();
 }

$code = $_POST['email'];

$DATA = $con->prepare("INSERT INTO users (code) VALUES (:code)");
$DATA->bindValue(':code', $code);
$DATA->execute();
?>

Answer 1

It depends on what you're looking to be safe.

If you're worried about the bobby tables case, you're fine. However, there are many risks that come with user input cases.

For such a simple script, little can go wrong. However, below is only one concern that can happen (however it is probably you're most threatening):

Cross-site Scripting (XSS)

Cross-Site Scripting (XSS) attacks are a type of injection, in which malicious scripts are injected into otherwise benign and trusted web sites. XSS attacks occur when an attacker uses a web application to send malicious code, generally in the form of a browser side script, to a different end user. The end user’s browser has no way to know that the script should not be trusted, and will execute the script. Because it thinks the script came from a trusted source, the malicious script can access any cookies, session tokens, or other sensitive information retained by the browser and used with that site. These scripts can even rewrite the content of the HTML page. (OWASP)

Basically, without validating what comes out of your data source, someone could input something nasty, and therefore giving anyone else who sees that data something nasty.

So to best protect yourself, sanitize any output from the data source and know the various attacks. Here's a good read on SO.

---

Also, it'd be great if you could format your code so it's more consistent and readable. Such as giving appropriate spacing around operators, correct indentations, and simply better variable names (why shorten $dbname, why CAPS $DATA?).

Answer 2

For a first try, this is basically OK. Of course, AlexL's concerns are all valid, and when you get to writing production code, you simply have to take all the things he mentioned into account. The golden rule of web applications is, after all, never trust the network.

However, I just though I might add a couple of tips and tricks, and point out a couple of gotcha's to complete Alex' answer:

PDO's methods can all cause errors, but by default, PDO will not throw exceptions when calling stuff like PDO::prepare. The constructor takes a fourth argument (an array), which allows you to configure the connection a bit more. Use that to your advantage. Here's how I usually create new instances of PDO:

$db = new PDO(
    'mysql:host=127.0.0.1;port=3306;dbname=myDb;charset=UTF8',
    $userName,
    $pass,
    array(
        PDO::ATTR_ERRMODE            => PDO::ERRMODE_EXCEPTION,
        PDO::ATTR_ORACLE_NULLS       => PDO::NULL_NATURAL,
        PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_OBJ,
        PDO::MYSQL_ATTR_INIT_COMMAND => "SET NAMES 'UTF8'"
    )
);

Now the what's and why's:

• 127.0.0.1: instead of localhost, I use the localhost IP, to avoid conflicts during name resolution that have been known to slow you down a lot. It's to do with IPv4 vs IPv6 conflicts and MySQL's settings
• port: Even if I'm connecting through the default port, I consider it good form to be complete, and provide PDO with as much information as it needs to connect to the DB. This includes the port, obviously.
• UTF8: Always specify the charset, wherever and whenever you can. the DSN string allows you to do just that, seize the opportunity. I repeat this in the attributes:
• PDO::MYSQL_ATTR_INIT_COMMAND => "SET NAMES 'UTF8'": Again, better to be safe than sorry: we can't be sure we're only sending ASCII chars over.
• PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION: Force PDO to throw exceptions on error. This helps debugging a lot, and is indispensable when working with transactions.
• PDO::ATTR_ORACLE_NULLS => PDO::NULL_NATURAL: If a field is NULL, there are 3 possible behaviours for PDO:
  • convert to empty string (which makes it hard to distinguish between a NULL and an actual empty string)
  • Convert NULL values to an actual string "NULL". That's just awful in every way, IMHO
  • Natural nulls, which I use: no conversion. An empty string is an empty string, a null is null.
• PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_OBJ: determine what the return type of PDOStatement::fetch calls will be if no constant is provided:

For example:

$row = $stmt->fetch();//<-- with settings above, $row is an instance of stdClass
$row = $stmt->fetch(PDO::FETCH_ASSOC);//row is assoc array.

This mainly saves you the hasstle of typing too much :)
As far as the PDO::ERRMODE_EXCEPTION is concerned, I've already mentioned transactions. So how do exceptions tie in with all this? Well, imagine you have to insert a couple of records in various tables, that are relational. If you inserted data into table 1, but then something went wrong (ie: malformed query) you have what is known as orphaned data. That's not what we want.
So we bundle all our INSERT queries into a transaction, and commit all the queries (ie save the new records) only when we're certain all the queries were successful. If at any point, one query failed, we can still undo or rollback the INSERT's:

$insert1 = $db->prepare('INSERT INTO tbl1 (foo, bar) VALUES (:foo, :bar)');
$insert2 = $db->prepare('INSERT INTO tbl2 (foo, more) VALUES (:foo, :more)');
try
{
    $db->beginTransaction();
    $insert1->execute(
        array(
           ':foo' => $foo,
           ':bar' => $bar
        )
    );
    $id = $db->lastInsertId();
    $insert2->execute(
        array(
            ':foo'  => $id,
            ':more' => $more
        )
    );
    $db->commit();//if everything worked, save the inserts
}
catch (PDOException $e)
{
    $db->rollback();//don't save changes, no table is altered
    $errorLog->error($e->getMessage(), $e->getCode());//log error
    //exit, or throw new RuntimeException...
}

You can even use the same insert statement multiple time in a loop, in a single transaction:

$stmt = $db->prepare('INSERT INTO foo (random) VALUES (:rand)');
try
{
    $db->beginTransaction();
    for ($i=0;$i<100;++$i)
    {
        $stmt->execute(
            array(
                ':rand' => mt_rand(1,10)*(i+1)
            )
        );
        $stmt->closeCursor();//close stmt, so we can reuse it
    }
    $db->commit();
}
catch (PDOException $e)
{
    error_log($e->getMessage());
    exit('Oops');
}

Spend some time browsing through the PDO manual pages, look at the examples and read the docs for all the methods I used here that you don't already know.