Protecting against Mysql Injection with PHP escapestring (any really good way of doing this?

Question

Possible Duplicate:
How to prevent SQL injection in PHP?

I wanted to ask a few questions about protecting against sql injection. From what I've been reading I keep coming across these three things:

• stripslashes
• which is used in conjunction with magic_quotes_gpc
• mysql_real_escape_string (or mysqli I suppose in the newer php?)

The question is, should I be using both of these or will real_escape_string suffice?

For instance I have this line of code pertaining to a registration page (which I know for a fact is vulnerable as sqli helper let me find everything about my database :( as I've not implemented any of the above yet):

    if(isset($_POST['submit'])){
    //cleanup the variables
    $username = ($_POST['username']);
    $password = ($_POST['password']);
    $email = ($_POST['email']);
    $username = sanitise($username);
    $password = sanitise($password);
    $email = sanitise($email);
    //quick/simple validation
    if(empty($username)){ $action['result'] = 'error'; array_push($text,'Please type in a username'); }
    if(empty($password)){ $action['result'] = 'error'; array_push($text,'Please type in a password'); }
if($action['result'] != 'error'){
        $password = md5($password); 
        //add to the database
        $add = mysql_query("INSERT INTO Users VALUES(NULL,'$username','$password','$email',0, 'First', 'Last', 'Phone Number Here', '...', 'Something about me...')");
        if($add){
            //get the new user id
            $userid = mysql_insert_id();    
            //create a random key
            $key = $username . $email . date('mY');
            $key = md5($key);
            //add confirm row
            $confirm = mysql_query("INSERT INTO Confirm VALUES(NULL,'$userid','$key','$email')");   
            if($confirm){
                //include the swift class
                include_once 'swift/swift_required.php';
                //put info into an array to send to the function
                $info = array(
                    'username' => $username,
                    'email' => $email,
                    'key' => $key);
                //send the email
                if(send_email($info)){
                    //email sent
                    $action['result'] = 'success';
                    array_push($text,'Thanks for signing up. Please check your e-mail for confirmation.');
                }else{
                    $action['result'] = 'error';
                    array_push($text,'Could not send confirmation e-mail');
                }
            }else{
                $action['result'] = 'error';
                array_push($text,'Confirm row was not added to the database. Reason: ' . mysql_error());
            }
        }else{
            $action['result'] = 'error';
            array_push($text,'User could not be added to the database. Reason: ' . mysql_error());
        }
    }
    $action['text'] = $text;
}
?>

I thought my sanitisation function would help things - got it online, however it would appear it is a bit useless. Or perhaps it only helps against cross site scripting. Here it is:

 function cleanInput($input) {
        $search = array(
            '@<script[^>]*?>
.*?</script>@si',   // Strip out javascript
    '@<[\/\!]*?[^<>]*?>@si',            // Strip out HTML tags
    '@<style[^>]*?>.*?
    </style>
@siU',    // Strip style tags properly
    '@<![\s\S]*?--[ \t\n\r]*>@'         // Strip multi-line comments
    );
    $output = preg_replace($search, '', $input);
    return $output;
    }
function sanitise($input) {
    if (is_array($input)) {
    foreach($input as $var=>$val) {
    $output[$var] = sanitise($val);
    }
    }
    else {
    if (get_magic_quotes_gpc()) {
    $input = stripslashes($input);
    }
    $input  = cleanInput($input);
    $output = $input;
    }
    return $output;
}

Would you suggest that function is useless?

If so, how would I go about securing the original bit of code? Namely:

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

Answer 1

use prepared statements, PDO or mysqli i personally prefer PDO, but both will do the job.

Answer 2

Whatever you do, Read about injection here. As you can see on this site, prepared statements are the way to go, preferably using PDO:

$stmt = $pdo->prepare('SELECT foo from db.bar WHERE foobar = :something;');
$stmt->execute(array(':something' => $_POST['something']));

There's no need to rely on deprecated features like magic-quotes or, in fact, anything in the mysql_* extension for that matter; as the latter is being deprecated entirely, and will eventually be removed from the language all together (hopefully some time soon).

In case you (or somebody else) is wondering why I think PDO is to be preferred:

• It supports multiple drivers (MySQL, MSSQL, PostrgreSQL,... ) full list here
• Its OO API is just more in-tune with the time, whereas mysqli_* offers a procedural API, too. This is regarded as a plus by some, mainly those who aren't to familiar with OOP, but you'll have to learn sooner or later.
• Personally, I get the impression that PDO is more widely used, and since PHP is an open-source product, it's what the community uses that matters: it'll be supported for a longer period of time, it'll be tested better (and by more people) and if you're stuck, there's a lot more peer-support.
• PDO emulates prepares, you can turn this off but that'll slow you down. Also, when emulating, PDO takes a closer look at the placeholders you're using. If, for example some wacky driver doesn't support named placeholders, you can still use them, but PDO will replace them with ?, or as the man pages put it "something more appropriate". All in all, PDO is quite clever :)

Answer 3

None of the functions you listed are connected to SQL injections :)

• stripslashes are just to recover from magic quotes
• magic quotes is a mis-feature, already removed from language
• mysql_real_escape_string (the way you're using it) - just a manual replica of magic quotes

as a matter of fact, escaping (done by magic quotes and real_escape) is only a fraction of a real formatting required for the properly built query. One have to follow whole set of rules, not just single one "escape everything and you're fine".

Nevertheless, there is nothing essentially wrong in escaping, as long as it is used in it's place, not as a magic wand protecting you from injections. For example, PDO is using it internally all the way.

Please note, that "use prepared statements" is not hte magic wand either, but rather the same mantra as "escape everything" - insufficient and unsafe as well.

Some explanations I posted before

Would you suggest that function is useless?

I would suggest that this function is quite harmful

Some explanations from the similar questiuon

any really good way of doing this?

Always use a placeholder to add a dynamical part to the query. It is safe and can be very convenient to use. Please note that prepared statements is not the only way to use placeholders and apparently not the most reliable one.

Also, bear in mind that using raw API calls in the application code, be it PDO, mysqli or mysql, are equally bad. One have to use an abstraction library to handle their SQL, not raw API functions.

Also, bear in mind that most of people who propagate PDO, never ever used it a real project, or at least some of it's "benefits". They actually just repeat the same text they just heard from others ;)

• Say, "multiple database support" is not the thing that used frequently, and of course it is not that easy as just change the DSN string. It is quite toilsome task to switch databases, and different API functions would be least problem.
• Or whoever who says "turning off emulation will slow you down" just never ever tried it in real :)
• Or, ask someone who says "PDO is quite clever" to create a simple query with IN clause populated from an array ;)