0

Given 1000s mysql queries in string format, could there be a way to analyze and remove any SQL injections from these strings before running the query?

one idea i had was to check the string for common words/phrases that are used in an sql injection which are never used in the application running the queries. If found, don't run the query and alert the admins.

David
  • 16,246
  • 34
  • 103
  • 162
  • 4
    Short answer: No, not reliably. It's really better to go through each query and sanitize the values properly (or use parametrized queries if that helps reduce the workload?) – Pekka Nov 02 '11 at 15:39
  • 2
    Like an `ON ERROR RESUME NEXT` but then used for good? `ON INJECTION DENY ACCESS`? – Konerak Nov 02 '11 at 15:42
  • 3
    You should also provide an example how you do it right now. If for example you already create the SQL in one string, a parser that needs to "fix" it, will run into the same problems like the db server. On side of the db server it's called sql-injection then, within your application just a flaw of your parser. – hakre Nov 02 '11 at 15:42
  • Such a function would be magic_quotes³ and once more the road to hell would be paved with good intentions. – VolkerK Nov 02 '11 at 15:52
  • 2
    `ON I MADE A MISTAKE TELL ME`. No. – Lightness Races in Orbit Nov 02 '11 at 15:53

2 Answers2

2

mySQL encourages the use of mysql_real_escape_string to sanitize queries.

Escapes special characters in the unescaped_string, taking into account the current character set of the connection so that it is safe to place it in a mysql_query(). If binary data is to be inserted, this function must be used.

mysql_real_escape_string() calls MySQL's library function mysql_real_escape_string, which prepends backslashes to the following characters: \x00, \n, \r, \, ', " and \x1a.

This function must always (with few exceptions) be used to make data safe before sending a query to MySQL.

PS: Here's a good answer to the addslashes/mysql_real_escape_string debate: http://www.sitepoint.com/forums/showthread.php?337881-addslashes()-vs-mysql_real_escape_string()...the-final-debate&s=7cabb6e5fd909f2c787d47cd01471dfb&p=2439889&viewfull=1#post2439889

Korvin Szanto
  • 4,531
  • 4
  • 19
  • 49
  • 2
    You have to pass each single parameter to mysql_real_escape_string, not a complete query. Also it is "only" designed for string literal parameters, not for all the other possible "moving parts" a query may contain. – VolkerK Nov 02 '11 at 15:49
  • You're right, I didn't explain this enough, he needs to rewrite the queries, that's the most practical way. – Korvin Szanto Nov 02 '11 at 16:07
1

one idea i had was to check the string for common words/phrases that are used in an sql injection which are never used in the application running the queries. If found, don't run the query and alert the admins.

Extremely bad idea.

The solution is to use PDO: http://php.net/manual/en/ref.pdo-mysql.php

And pass all $vars using parameters.
If you have to use dynamic SQL, make sure you check your injected stuff against a whitelist.
See this question on how to safely do that: How to prevent SQL injection with dynamic tablenames?

Now you are save.

If you cannot use PDO, use mysql_real_escape_string().
Here's an example:

$var = mysql_real_escape_string($_POST['unsafe_value']);
$query = "SELECT * FROM user WHERE username = '$var' ";
// The quotes are vital, without them         ^    ^  
//you will may get syntax errors and mysql_real.... will not protect you!

You can only use this for parameters not for table, column, database names or SQL constructs, also remember to always quote your '$vars'.
For anything but values you must use whitelisting.
Note that addslashes does not work and should not be used!
For integer values you can use intval(), but I advice against it, it saves no time, makes your code harder to read and it breaks the rule to always use mysql_real_escape_string().

Community
  • 1
  • 1
Johan
  • 74,508
  • 24
  • 191
  • 319