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.