sql injection

[brown2005]

function make_safe($variable) 
{

  $variable = addslashes(trim($variable));

  return $variable;

}

$username = make_safe($_POST['username']);
$password = make_safe($_POST['password']);

 

which just reading about this and got the above code to prevent it.. is this the best way? or does anybody have a better way?

 

[Daniel0]

The MySQL extension has it's own function to deal with that: mysql_real_escape_string()

 

Instead you should consider using PDO instead and prepared statements though: http://php.net/pdo

[cunoodle2]

Looks pretty good.  Just make sure you do a little pre-cleaning on those variables.  This is all just overkill but seriously the more the better..

 

<?php

// Empty Username And Password Variables
$username = '';
$password = '';

if (isset ($_POST['username']) && $_POST['username'] != '')
$username = $_POST['username'];

if(isset ($_POST['password']) && $_POST['password'] != '')
$password = $_POST['password'];

$username = make_safe($_POST['username']);
$password = make_safe($_POST['password']);

//Perform SQL..
$login = mysql_query("SELECT * FROM users WHERE `username` = '$username' AND `password` = '$db_password'");

if (($result = mysql_query( $query )) == NULL) 
{
   echo mysql_error();
   exit;
} 

else 
{

    if (($num = mysql_num_rows( $result )) > 1) 
    {
           echo 'Problems here, There is more then one match';
           exit;
    } 

    else if ($num == 1) 
    { 
          echo 'Match Found!'; 
    } 

     else { echo 'Sorry No Match'; }

?>

}

[cunoodle2]

Instead you should consider using PDO instead and prepared statements though: http://php.net/pdo

 

I've seen you mention this a few times on here.  I'm honestly not getting the whole PDO thing.  Could you post a bit of sample code on here?  I tried looking in tutorials but they are not up.  Also mods should consider a big tutorial section on mysql injection as it seems to just be a massive repeated topic on here

[Daniel0]

Sure, PDO is "PHP Data Objects". It provides a sort of consistency across the different DBMSs (how do you pluralize that?). This has the advantage of being easily able to change the DBMS should you like to do that.

 

When you create a prepared statement you'll create sort of like a template for a query and using either markers with names or just question marks as placeholders/variables. You'll then execute a prepared statement. By executing you are also passing the remaining data, i.e. the variables/placeholders, and thereby creating a complete query which will be run. When this is being executed you'll PDO will take care of adding quotes around the value if necessary and it will escape it and make it safe. On the statement object you can then call methods like fetch(), fetchAll(), etc.

 

Here is a short example:

<?php
// connect
$pdo = new PDO('mysql:host=localhost;dbname=test', 'username', 'password');

// create a prepare statement
$statement = $pdo->prepare('SELECT * FROM users WHERE username = ? LIMIT 1');

// execute it and send the username to it. then fetch the row to an array
$user_info = $statement->execute($_GET['username'])->fetch();

print_r($user_info);
?>

[dbo]

In regards to this post:

 

Validate your inputs first, make sure your users play by your rules. This means they use your formats for phone numbers, dates, etc.

 

Only after you have validated your input should you worry about sending it to the db. This is when you should escape it (mysql_real_escape_string) or equivalent. As Daniel0 pointed out, prepared statements are generally considered the most safe method as it will do this for you.

 

To Daniel0:

How does pdo compare to pear::DB? I've only tinkered with pear::DB and have never used pdo... was planning on leveraging it for my next project, mainly for the database abstraction layer. Just curious on your take.

[Daniel0]

To Daniel0:

How does pdo compare to pear::DB? I've only tinkered with pear::DB and have never used pdo... was planning on leveraging it for my next project, mainly for the database abstraction layer. Just curious on your take.

 

I haven't used that library, but seeing as it's written in PHP and PDO is written in C, PDO ought to be faster than the PEAR library you are referring to. I wouldn't use it if I were you unless it provides something in addition to PDO that would take long time for you to recreate.

[cunoodle2]

To Daniel0:

A few more questions....

 

You said "When this is being executed you'll PDO will take care of adding quotes around the value if necessary and it will escape it and make it safe."

So does that mean with PDOs that you do not have to even bother with addslashes and/or mysql_real_escape_string??  How exactly do you handle things?

 

Also how is this...

$statement = $pdo->prepare('SELECT * FROM users WHERE username = ? LIMIT 1');

$user_info = $statement->execute($_GET['username'])->fetch();

 

Any different from this..

$username = make_safe($_POST['username']);

$login = mysql_query("SELECT * FROM users WHERE username = $username LIMIT 1");

 

To me they appear as though they would execute in the exact same fashion and return the exact same results.  I do want to say that I am not attempting to critique your work.  PDOs are just new to me and just trying to learn the best ways to secure your/my code.

[Daniel0]

You're correct in saying that you wont have to bother with manually securing the data. An added benefit of using prepared statements instead is that prepared statements is a sort of template which has been parsed. This means that it'll decrease the execution time should you need to run the query multiple times as it only has to be parsed once, but executed multiple times with different arguments whereas using non-prepared statements the query will have to be parsed multiple times. Note that you can in fact use PDO::query() to run queries in the way mysql_query() does. If you're exclusively using prepared statements then you are also completely eliminating the aspects of SQL injection as it will all be taken care of automatically. If I remember correctly, then all vendor specific DBMS extensions will also be disabled by default in PHP 6 and PDO will be enabled by default (it is enabled by default in PHP 5.1+ as well). PDO being object oriented also makes it easier to extend and easier to encapsulate your code, which is in my opinion a great thing.

 

I hope that answered your questions...