Preventing SQL injection using ONLY php

Question

The think is that i have a complete working website with many calls to the MySQL server and doing some research on this site i saw that making my querys in this form:

$query = sprintf("SELECT * FROM users WHERE user='%s' AND password='%s'",
            mysql_real_escape_string($user),
            mysql_real_escape_string($password));

I can solve the security issue, but, as i said, i have many calls to the MySQL server, and the best way (in my case) to solve the problem is going directly to the vars im passing to the query but whitout using a MySQL function because im out of the query. Let me explain it, i have this:

mysql_query("SELECT * FROM `post` WHERE id=" . $_GET['edit']);

I cant do modifications to this query because i have a lot of this in all my code, insted i preefer to check for injections on the var, $_GET['edit'].

How can i using pure PHP check for SQL injections on the variables of the querys? Like:

$_GET['edit']=freehack($_GET['edit']);

Answer 1

Don't do it this way. By replacing the value of your $_GET parameters with "safe" versions, you are contaminating your input data, which you may need for other places.

Only escape data when you need to use it on the database layer. It will only take you a little time to fix your queries, and will save you a ton of headache in the long run.

In any case, what you are doing is still not secure! See: PHP: Is mysql_real_escape_string sufficient for cleaning user input?

You really should be using prepared queries with PDO. Also, you should be checking your user input for validity before using it in a query.

Answer 2

"I cant do modifications to this query because i have a lot of this in all my code" is the wrong attitude when talking about security. What you have there is a major design problem that opens you up to all sorts of security issues. Even if you do something like the filter method you describe at the end there, you can't be sure you'll cover every case.

You should really be using some sort of database access class to query the DB instead of making random calls like this throughout your code. This way you can write the sanitization code once and can be absolutely sure it is called everywhere. Refactoring is worth it for the added security you'll get.

Answer 3

I think you could wrap your query inside PDO.

$unsafe = "SELECT * FROM `post` WHERE id=" . $_GET['edit'];
$DBH->quote($unsafe); // makes query safe.

where $DBH = new PDO("mysql:host=$host;dbname=$dbname", $user, $pass);

Then you have to write some sort of scripts to do the replacements.Then again I really think you should be rewriting your code from scratch because it is really ugly. Properly unit tested, PDO, CSRF-protection, OOP etc.

Answer 4

I would recommend using the PDO interface, or the MySQLi inteface as both support using prepared queries. Using prepared queries is the only way to effectively and consistently protect yourself again SQL Injection attacks. I personally recommend PDO over mysqli as it provides a database agnostic interface to your database, in case you ever need to switch databases. Even if you never need to switch databases, it's better to only have to learn 1 interface, in case you need to work an another project using a different database.

Answer 5

I wouldn't go with a "magic" function and expect it to clean the variables. This won't solve all the edge case and it will still be vulnerable.

A good example of this is your 2nd query. It's still vulnerable to SQL injection even if you mysql_real_escape_string the $_GET['edit'] variable.

What you need to do is validate all the data you receive and check if it's the kind of data you expect.

if (SomeValidator::validateInt($_GET['edit'])) {
    // OK, we continue //
} else {
    // Display error, but don't continue ! //
}

Sanitizing is only there make sure the data will be properly display and won't cause problem. You shouldn't rely on sanitizing for the validation of your data.

If you want to get the validation and the sanitizing done right you can use the ESAPI for PHP.

Answer 6

1. Avoid having the same query repeated in different places across your application - as things grow you have multiple versions to maintain and they almost invariably get out of sync. Prepared queries sounds good to me (I'm not really a PHP guy), or if you have to go another way set up a central reference library and store your queries in there, then reference them when you need them in your app.

2. Don't store encoded data, it's too easy to forget which variables are encoded and which aren't, plus you end up in trouble as soon as you have to use your data for a purpose with a different encoding. Encode as late as possible in the process, ideally when it's being put into the final situation where it needs encoding so that a quick inspection can show it's being done.

3. If you have to.... SQL Injection is significantly a type safety issue. If you know you're expecting an integer parameter, "1; drop table users;--" isn't valid input even though it doesn't have any danger characters or escape sequences in it. Don't just check for busting out of strings or whatever, make sure that when you want a specific type, you get that and other input throws an error.