It’s amazing – I’ve been going around, Googling for anything with “index.php?id=”…and that’s really all it takes. Now, granted, SQL Injection isn’t new, and a lot of the top hits have taken some steps to protect themselves, but if you go deep – like, Google search page 23 deep – you’ll find ones that break if you put a semi-colon after the id # – and if it breaks, it’s vulnerable.
So, here’s my first tip on preventing SQL Injection – when you’re asking for an ID number, make sure it’s a number, and nothing else. Also consider using prepared statements – database wrappers like MDB2 for PHP make this easy.
Check this out – this might be how I would have done it 3 years ago:
<?php //Assume we're already connected to a MySQL database... $id = $_GET['id']; $result = mysql_query('SELECT * from pages where id='.$id); if (!$result) { die('Invalid query: ' . mysql_error()); } ... //Code to print out my result to the page ?>
I’d do it this way now:
Note: My use of MDB2 might be a little rusty – I haven’t tested this code, and I usually compose RowDataGateway objects with MDB2 to represent my data. So pay more attention to the structure than the actual syntax.
<?php require 'View.php'; require 'MDB2.php'; //An excellent DB layer from the PEAR libs //Code to set $mdb2 as our DB connection variable //See http://pear.php.net/package/MDB2 for details $id = $_GET['id']; try { if(!is_int($id)) { //ID wasn't an int, it's no good, let's bail throw new Exception('Could not recognize the id that you passed'); } //ID was an int, let's see if we can find the record $sql = 'SELECT * from pages where id=:id"; $statement = $mdb2->prepare($sql); $statement->bindParam('id', $id); $result = $statement->execute(); if(PEAR::isError($result)) { //Uh oh - our result was an error on the PEAR library level throw new Exception('There was an error communicating with the database'); } //Insert the database result into the view, render, and die. $content = new View('templates/page.tpl', array('page' => $result->fetchOne())); $content->render(); die; } catch(Exception $e) { //We must have caught an exception - put this into our //error page template with the error message, render, die. $content = new View('templates/error.tpl', array('message' => $e->getMessage())); $content->render(); die; } ?>
Yes, it’s quite a bit more code. But I feel safer just looking at it.
Did I miss anything on this? Please post a comment if you notice that I’ve left a gaping hole. Learning is good.