1

In response to a previous question I had posted,there was a response that I was opening up my database to SQL injection. The code is given below :

    <?php


        $firstname = stripslashes(strip_tags($_POST['firstname']));

        $lastname = stripslashes(strip_tags($_POST['lastname']));
        $email = stripslashes(strip_tags($_POST['email']));
        $title = stripslashes(strip_tags($_POST['title']));
        $organization = stripslashes(strip_tags($_POST['organization']));


    $pdbHost = "localhost";
    $pdbUserName = "******";
    $pdbPassword = "******";
    $pdbName     = "db1080824_emails";



    //  Connect to mySQL
    $conlink = mysql_connect($pdbHost, $pdbUserName, $pdbPassword);
    if(!$conlink) {die('Unable to connect to '.$pdbHost);}
    if (!mysql_select_db($pdbName, $conlink)){die('Cannot find database '.$pdbName);}

    //SQL query

        $SQL2="INSERT INTO  `db1080824_emails`.`emails` (`record_id` ,`firstname`,`lastname`,`email`,`title`,`organization`)VALUES (NULL ,  '".$firstname."',  '".$lastname."',  '".$email."',  '".$title."',  '".$organization."')";

        mysql_query($SQL2);
    //  Connect to Closing the connection
    mysql_close($conlink);
?>

A suggestion was that I can do server side validation checks for '/^[A-Za-z0-9]/' to ensure no sql injection might happen but is that sufficient or is there best practice I should follow to ensure data sanitization ?

5
  • en.wikipedia.org/wiki/SQL_injection - read more on how it's done and how to "undo"
    – Joseph
    Commented Mar 4, 2012 at 7:55
  • 1
    Use only ever prepared statements. Period.
    – knittl
    Commented Mar 4, 2012 at 7:59
  • 3
    First step: stop using mysql_query and use a modern API like PDO or mysqli. Both support prepared statements -- which, when used properly, go a long way toward preventing SQL injection.
    – cHao
    Commented Mar 4, 2012 at 8:00
  • But how do I use prepared statements when the information is coming from a form ?
    – Mervin
    Commented Mar 4, 2012 at 8:02
  • exactly the same way as before Commented Mar 4, 2012 at 10:34

3 Answers 3

3

Use prepared statements. Seriously. An easy way to do this, is to use the database wrapper PDO of PHP.

$firstname = $_POST['firstname'];
$lastname = …;
…

$db = new PDO('mysql:host=hostname;dbname=dbname', 'username', 'password');
$stmt = $db->prepare('INSERT INTO  `db1080824_emails`.`emails` (`firstname`,`lastname`,`email`,`title`,`organization`)
  VALUES (:firstname, :lastname, :email, :title, :organization)');
$stmt->execute(array(
  ':firstname' => $firstname,
  ':lastname' => $lastname,
  ':email' => $email,
  ':title' => $title,
  ':organization' => $organization));
1

Use mysql_real_escape_string() to prevent SQL injection. Only this function is enough.

And, what @knittl said, using prepared statements is also a very good method to prevent it. But the normal mysql_* libary doesn't support that. You need a libary as PDO or MySQLi for things like that. I suggest you to switch to PDO or MySQLi, because in higher PHP versions the mysql_* libary be would deprecated.

Some other tips to improve your code:

  • don't use die(). If you make a mistake you won't die, so why a computer? It is better to handle errors nice and put them on place you want them.
  • mysql_close() isn't needed. PHP close every connection at the end of the execution.
  • use error handling on the query, if the query returns false there is a problem. Also check with mysql_affected_rows() or mysql_num_rows if the query has done something.

I order to answer the comments on this answer. You should use mysql_real_escape_string only for strings. If you are using mysql_real_escape_string() be sure you have put quotes (') around the string in the query:

$query = "SELECT foo FROM bar WHERE name = '".mysql_real_escape_string($_POST['name'])."'";

If you use intergers or any other number you should use typecasting and not the escape function:

$query = "SELECT foo FROM bar WHERE id = ".(int) $_POST['id'];
3
  • You can use mysql_real_escape_string, but only if you know what you're doing. i.e. only use it for values enclosed in quotes (but for every such value), have the right encoding set on the client side (PHP) as well as on the server/connection side (mysql). There's just so much where you can miss a little thing and be screwed again.
    – knittl
    Commented Mar 4, 2012 at 8:11
  • this function is NOT to prevent any injections. Go figure Commented Mar 4, 2012 at 10:34
  • Thank you for the comments, I've updated my answer with some information. @Col.Shrapnel why is this function not for preventing SQL injection? From php.net: If this function is not used to escape data, the query is vulnerable to SQL Injection Attacks.
    – Wouter J
    Commented Mar 4, 2012 at 12:35
0

I've been using PDO, I believe that this protects from injections:

$db_user = "****";
$db_pass= "****";
$dsn = "mysql:dbname=yourdatabasename; host=localhost";
$dbh = new PDO($dsn, $db_user, $db_pass);

$sql = 'INSERT INTO
        emails(firstname, lastname, email, title, organization)
        VALUES(:firstname,:lastname,:email,:title,:organization)';
$data = array(':firstname'=>$firstname,
              ':lastname'=>$lastname,
              ':email'=>$email,
              ':title'=>$title,
              ':organization'=>$organization);

$sth = $dbh->prepare($sql);
$sth->execute($data);
0

Not the answer you're looking for? Browse other questions tagged or ask your own question.