0

Possible Duplicate:
How prepared statements can protect from SQL injection attacks?

If I'm using $_GET with PDO do I still need to escape it? My understanding is that this is immune to SQL injection, however I still feel uneasy about not escaping it. So could someone please look at this little block of code and tell me if it is secure?

<?php
$hostname = 'localhost';
$username = 'root';
$password = 'root';
$database = 'database';
try {
    $dbh = new PDO("mysql:host=$hostname;dbname=$database", $username, $password);
    $dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
    $stmt = $dbh->prepare("SELECT * FROM comments WHERE pid = :pid");
    $pid = $_GET['pid'];
    $stmt->bindParam(':pid', $pid, PDO::PARAM_STR);
    $stmt->execute();
    $result = $stmt->fetchAll();
    }
catch(PDOException $e)
    {
    echo $e->getMessage();
    }
    $stmt->execute();
    echo $stmt->rowCount();
$dbh = null;
?>

Again, it's the $_GET I'm concerned about. Any help is appreciated, thank you.

Community
  • 1
  • 1
Ian
  • 1,740
  • 6
  • 21
  • 37
  • 3
    If you needed to escape variables for prepared queries - then prepared would be almost completely useless – zerkms Feb 23 '12 at 02:33
  • In other words, stuff whatever you want into a prepared statement placeholder - the DB will take care of the escaping for you, regardless of where that data came from. – Marc B Feb 23 '12 at 02:36
  • That's quite funny uneasiness, to someone who understands that (unlike prepared statements) escaping doesn't make your "data" "safe" – Your Common Sense Feb 23 '12 at 02:40
  • 1
    I don't understand your statement... nowhere in here did I say that escaping it made data safe... so what are you ranting about? – Ian Feb 23 '12 at 03:04
  • I thought it's rather clear. You "feel uneasy about not escaping it" and your concern is security. So, the only conclusion one can make is that you think escaping makes data secure. – Your Common Sense Feb 23 '12 at 03:17
  • So... you're saying that escaping has absolutely nothing at all to do with security...? That's like me saying I'm concerned about home security and feel uneasy because I don't have locks on my door, then you start bitching about how I don't have an alarm system, an attack dog, and a 12 gauge shotgun... I never said that escaping is all I have to do for security, that was entirely an assumption and leap of logic on your part. – Ian Feb 26 '12 at 06:13

1 Answers1

2

Yes, the prepared statement feature does what it says. But since you asked, let's be clear that it's not the end of the story. I'm looking at the OWASP Top Ten Application Security Risks 2010.

For example:

  • Is every remote user authorized to access data associated with every PID? If not, failing to check that the user is authorized is a clear example of OWASP 2010-A4-Insecure Direct Object References.
  • You're probably not serious about hardcoding the password in cleartext, because that is a clear example of OWASP 2010-A7-Insecure Cryptographic Storage.
  • You don't say what you might do with $stmt apart from echoing the rowcount, but of course if you display any content from the database you'll be careful to escape HTML entities first. Otherwise you would create a clear example of OWASP 2010-A2-Cross-Site Scripting (XSS).
  • By the way, it's generally better to specify columns (or aggregate functions) explicitly rather than to "SELECT *".
minopret
  • 4,656
  • 18
  • 33