Does this PHP function protect against SQL injection?

Question

I have this function I'm using and I want to be sure that it fully protects against SQL injection attacks:

function MakeSafeForQuery($string)
{
    // replace all of the quote
    // chars by their escape sequence

    $ret = str_replace("\\","\\\\",$string);
    $ret = str_replace("'","\\'",$ret);
    $ret = str_replace("\"","\\\"",$ret);

    return $ret;
}

Am I missing anything serious?

Edit: I'm using MySQL by the way.

Are you using MySQL? If so, I'd suggest using PHP's mysql_real_escape_string() function. — Eric G, Mar 31 '10 at 05:04

score 11 · Accepted Answer · edited Dec 20 '11 at 09:03

In GBK, 0xbf27 is not a valid multi-byte character, but 0xbf5c is. Interpreted as single-byte characters, 0xbf27 is 0xbf (¿) followed by 0x27 ('), and 0xbf5c is 0xbf (¿) followed by 0x5c (\).

How does this help? If I want to attempt an SQL injection attack against a MySQL database, having single quotes escaped with a backslash is a bummer. If you're using addslashes(), however, I'm in luck. All I need to do is inject something like 0xbf27, and addslashes() modifies this to become 0xbf5c27, a valid multi-byte character followed by a single quote. In other words, I can successfully inject a single quote despite your escaping. That's because 0xbf5c is interpreted as a single character, not two. Oops, there goes the backslash.

This type of attack is possible with any character encoding where there is a valid multi-byte character that ends in 0x5c, because addslashes() can be tricked into creating a valid multi-byte character instead of escaping the single quote that follows. UTF-8 does not fit this description.

To avoid this type of vulnerability, use mysql_real_escape_string()

http://shiflett.org/blog/2006/jan/addslashes-versus-mysql-real-escape-string

outis · Answer 2 · 2010-03-31T09:55:32.820

4

These days, you should use prepared statements, whose parameters aren't vulnerable to injection, rather than sanitizing functions. PDO supports them. Read "Writing MySQL Scripts with PHP and PDO" for more information.

Note that only scalar values can be parameterized with most DB drivers. Compound values (such as used in the IN (list) operator) and other parts of statements cannot usually be parameterized. For these, you still need to interpolate values into the statement. It's best not to ever use user input directly for anything other than values.

edited Mar 31 '10 at 09:55

answered Mar 31 '10 at 05:37

outis

75,655
22
151
221

Note that prepared statements won't help you with query identifiers. Nor any escaing function do. – Your Common Sense Mar 31 '10 at 06:03
@Col.: an important point, hence the (admittedly terse) phrasing "parameters aren't vulnerable". Answer updated to be more explicit. – outis Mar 31 '10 at 09:51

score 3 · Answer 3 · answered Mar 31 '10 at 05:05

To protect against SQL injections, you should not use a function you are writing yourself.

Instead, you should use a function that's provided by your database driver.

For instance :

The right way to escape strings depends on both the database system, and its configuration (which is why escaping functions generally require an active database connection) -- which means your function could/would work in most cases, but not necessarily always.

Escaping itself is not enough to protect anything – Your Common Sense Mar 31 '10 at 06:06 — Your Common Sense, Mar 31 '10 at 06:06

score 1 · Answer 4 · answered Mar 31 '10 at 05:04

1

Couldn't you just use mysql_real_escape_string or are you trying to do it without a database connection?

(Which seems to defeat the purpose....)

answered Mar 31 '10 at 05:04

Tyler Carter

60,743
20
130
150

I'm trying to see if I can write something that works WITHOUT using `mysql_real_escape_string`. Does what I have protect me from problems? – Nathan Osman Mar 31 '10 at 05:05
Why, George? If you take your car to a garage, would you want the mechanic to build his own tools and run-up your bill? No? Cause your clients/boss don't want YOU to waste THEIR time and MONEY. – Amy B Mar 31 '10 at 05:24
No, it won't protect you from problems. Proper escaping is a good first step. – Xorlev Mar 31 '10 at 06:27

score 0 · Answer 5 · edited May 23 '17 at 11:48

Definitely NO.

Not because of some exotic encodings you will never meet in your life, but because of the simple fact that escaping alone helps nothing.

Adding slashes is not like some divine intervention that instantly prevents scaring injection.

In fact, it works only if combined with quotes. If you quote your escaped data - you may consider yourself safe, even with your home-brewed escaping function (as long as your encoding either single-byte or UTF-8). However, sometimes we don't (or even can't) use quotes around our variables and such a place instantly becomes a security hole.

For the further explanations you may refer to my previous answer to the similar question: https://stackoverflow.com/a/8061617/285587

As for the accepted answer, just keep in mind that both mysql_real_escape_string() and PDO prepared statements has exactly the same vulnerability if used by itself, without taking required precautions. You have to properly set client encoding to make them work as intended.

So, in fact just use mysql_real_escape_string() will help you no more than your own function.

mysql_set_charset() have to be used with mysql_real_escape_string() and in PDO either emulation mode have to be turned off or encoding should be set in DSN

Does this PHP function protect against SQL injection?

5 Answers5

Definitely NO.