Development Protecting Against SQL Injection

Hey guys,

I have a web page that has a list of options that the user can click on, and it will then query the db based on the click. the links point back to the same page, with each link like this:

img.php?gal=Name

the page will then check if its set, then query the db:

PHP:

$SQL = " SELECT Name, Description FROM Images";                //default value
                
if ( isset($_REQUEST['gal']) )
{
    if ( $_REQUEST['gal'] !== 'All')
    {
        $SQL = " SELECT i.Name, i.Description FROM Images i, Galleries g WHERE (i.ID = g.ID) AND (g.Name ='" . $_REQUEST['gal'] ."')"; 
    }
}

The thing I'm worries about is someone putting in a bad parameter to inject SQL into the db. I had thought of testing to see if the perameter is already in the db, by getting a list of the galleries, and checking if the parameter is in that list, but I'm not too sure how to do this in php.

PHP:

$s = "SELECT Name FROM Galleries WHERE Name = '" .  $_REQUEST['gal'] . "' "
$ret = mysql_db_query($db, $s, $dbh);

(then test if the result set is greater than 0)

but this wouldn't work as it would still have SQL injection vulnerability.

any ideas would be great

thanks

 

Firstly, avoid using $_REQUEST, it's very very unsafe.

Set the forms to use method="post", then use $_POST array rather then $_REQUEST, that means that it won't be parsed in the url (the URL parsing is $_GET)

Easiest way to check for injection attacks is to use

PHP:

if(stristr(';',$_POST['gal']))
{
die('OI! Stop trying to hack me!');
}

 

Except for it'd only catch just one of a very few ways of attempting SQL injection, so don't do that...

GuardianStorm, your code is definitely vulnerable to SQL Injection. If you know all of the names of your galleries you can write some rudimentary protection, but it'd need to be modified every time you added, renamed or removed a gallery in your database.

The best thing to do is to change the way that you are using PHP to talk to your MySQL server. The MySQLi extension has 'built in' protection against SQL injection via use of prepared statements and isn't all that more difficult to use. See php.net/mysqli and php.net/mysqli_bind_param.

 

The really easiest way, is that if you're expecting a name, which has a maximum length of say 10 characters (or even use a lookup table with converts a number to the gallery) user regular expressions with "[0-9]{*,5}" which checks for just a input of 0-5 numbers, no more nothing else.

 

Would it not be easier and safer to use the gallery id instead of the gallery name, then using intval() to strip out unwanted data? Or limit the gallery names to a-z0-9 and use ctype_alnum() to check the name is valid?

 

Thanks, but as its not a form (my forms already use $_POST) i will use $_GET.

Thanks for the link, however as the server is not mine (i rent from Hosting-Unlimited) i dont know if they have mysqli installed or iff they can. is there a script i can run to determine if it is installed/enabled?

I could use the integer version, as the table is a Name-Value pair, so that would be a very viable option.

That might be coupled with DougEdey's suggestion, thanks.

Thanks everyone

 

Yep! Although i'd say it'll be easier for you to go down doug's route (integers rather than names) rather than move to mysqli.

But anyway, here's a script to tell you if they have mysqli or not:

Code:

<?php
echo function_exists('mysqli_connect') ? 'this server has mysqli' : 'this server doesn\'t have mysqli :(';
?>

 

thanks for the help again RTT, and unfortunately the server does not have MySQLi installed

I will implement the integer method as suggested by Doug, should be easy enough

thanks again everyone!

 

See, reg ex is brilliant.

 

Thanks for the help everyone, here is the code i am now using:

PHP:

if ( isset($_GET['gal']) && (is_numeric($_GET['gal'])) ) 
{
    $SQL = "SELECT i.Name, i.Description FROM Images i JOIN ImageGalleries ig ON i.ID = ig.ImageID WHERE ig.GalleryID = '" . $_GET['gal'] . "'";
}

If anyone can see problems with this, I am all ears

 

Can you do parameterized queries? Or is that not doable with PHP or with MySQL or your server? I don't know. I'll find out when I am more sober.
thanks you.

 

Don't re-invent the wheel

Use a PEAR based framework and let it do the work for you.

 

that's no way to learn!
do it by hand at first, move to library code later.

 

Guardianstorm: not a major one, but to improve efficiency only use doubles quotes (") to enclose a string if you want PHP to evaluate it (i.e. if you have any escape, \*, chars in there), for non evaluated, static strings, enclose them in single quotes (') not a biggie, but good habits survive a lifetime, when bad habits die slowly