There are many outstanding ways to secure applications against SQL-injections, for example this Q&A shows some ways How to prevent SQL injection in PHP?.
This question is about an already written application. We don't want to modify it in order to improve its safety. We want to know that the used method is vulnerable to injection attacks or not.
We need to know that this part of the code is the reason of leaking the data or not?
Here is a PHP function to eliminate some useless (for us) and maybe harmful characters:
function removeBadCharacters($s)
{
return str_replace(array('&','<','>','/','\\','"',"'",'?','+'), '', $s);
}
It throws out dangerous characters to avoid SQL injection attack (Don't worry about removed characters such as &<>/\"'?+
the application don't need them):
$x = removeBadCharacters($_POST['data']);
mysql_query("insert into table (x) values ('".$x."');");
mysql_query("select * from into where name = '".$x."';");
To be able to inject arbitrary SQL from the context of a string literal, that string literal needs to be left. This is only possible by introducing a string end delimiter, in this case a single '
, or by expand the a string literal to a preceding '
, e.g., by using the escapes character \
:
$a = '\\';
$b = ' OR 1=1 OR ';
$c = ' --';
$query = "SELECT * FROM t1 WHERE a='$a' AND b='$b' AND c='$c'";
// result:
// SELECT * FROM t1 WHERE a='\' AND b=' OR 1=1 OR ' AND c=' --'
// \_________/ \_______/
Now as your function removes any '
and \
, it seems to be impossible to leave or expand the string literal and thus not possible to inject arbitrary SQL.
However, since your function does not take the actual character encoding into account, it is possible to exploit this if the MySQL’s character encoding is GBK, similar to how it can be exploited when using addslashes
instead of mysql_real_escape_string
:
$a = "\xbf";
$b = " OR 1=1 OR ";
$c = " --";
$query = "SELECT * FROM t1 WHERE a='$a' AND b='$b' AND c='$c'";
// result:
// SELECT * FROM t1 WHERE a='縗 AND b=' OR 1=1 OR ' AND c=' --'
// \_________/ \_______/
So to play safe, use mysql_real_escape_string
or other proven methods to prevent SQL injections.
The very idea of removing whatever characters is utterly wrong.
That's what essentially wrong your approach.
You have to format your SQL literals properly instead of spoiling them.
Imagine this very site were using such a "protection": you'd were unable to post your question!
To answer your question literally - yes, under some circumstances it's very easy to inject. Just because the very idea of all-in-once sanitization is broken. PHP had a similar feature once, called "magic quotes". It was a hard lesson, but now it's got removed from the language at last. For the very reasons I told you:
- it does not make "data" "secure"
- it spoils your data instead
Every SQL literal have to be treated personally, according to its role.
Yes, this can be defeated using Unicode characters and manipulating character sets.
See https://security.stackexchange.com/questions/11391/how-did-anonymous-use-utf-16-ascii-to-fool-php-escaping for further details.
Your function doesn't deal with different encodings.
Don't try to come up with sanitation methods yourself, use something already made. In the case of mysql_*
, it would be mysql_real_escape_string
, however, you shouldn't use mysql_*
anymore, use PDO or mysqli instead.
See: How can I prevent SQL injection in PHP? for further details.