Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Preventing SQL injection using ONLY php

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']);
like image 306
DomingoSL Avatar asked Jan 26 '11 15:01

DomingoSL


3 Answers

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.

like image 90
Brad Avatar answered Sep 30 '22 15:09

Brad


"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.

like image 22
Wade Tandy Avatar answered Sep 30 '22 13:09

Wade Tandy


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.

like image 30
Alfred Avatar answered Sep 30 '22 15:09

Alfred