Is this use of str_replace sufficient to prevent SQL injection attacks?

Question

Edit if you plan on answering this question please at least read it. Don't simply read the title, then Google 'sql injection php', and paste the results as an answer

First, I'm well aware that there are lots of resources available on how best to prevent SQL injection, but my question is specifically about if very little effort can be enough.

An organization I contract for was recently told that partners' (PHP) websites developed by their previous contractor have been found to have major security issues (my personal favourite is by using the string 'Emergency' in a URL you can gain unauthenticated access to any page in the site...)

I have been asked to review the security of a PHP site and highlight any major issues. I know from previous experience with this site that the standard of coding is truly awful (e.g. huge swathes of code duplicated across pages with around 5% variation, hundreds of unused variables, $var = "yes" in place of booleans, SQL statements written into every single script, unsecure by default (some pages forget to authenticate users), etc). Reviewing the site is a painful reminder that there are some real morons in the world that call themselves developers.

Because of these code quality issues I want to only highlight the serious and semi-serious security problems. If I note every problem with the site my review will take weeks.

I am not an expert on SQL injection, but it's my understanding that you must be able to close an existing query's quoted string before injecting any other kind of statement. So is the following line of code sufficient?

"'".str_replace("'","''",$_POST['var_name'])."'"

I'm not interested in suggestions about improving or changing the code, just whether it's possible in this scenario for SQL injection via $_POST['var_name']. If you're not familiar with PHP str_replace does replace all instances of the first argument with the second, not just the first instance.

Any input much appreciated.

You might want to take a look at [this answer](http://stackoverflow.com/questions/3115559/exploitable-php-functions/3697776#3697776) by @Rook — Sean Vieira, Jul 22 '12 at 01:07
Personally I think the easiest is using PDOs. It might mean a bit more changes to the code but I consider it the "minimum effort" because as long as you use prepared statements, you're **guaranteed** to be protected against SQL injection. http://php.net/manual/en/book.pdo.php http://www.php.net/manual/en/pdo.prepare.php — Raekye, Jul 22 '12 at 01:20
Tom, I loved your question because it inspired mikemacintosh's answer, but a small adjustment to your question would clarify: Suppose the code did `"'".str_replace(array("\\","'"),array("\\\\","''"),$_POST['var_name'])."'"` (to double the backslash too)? — Dave Scotese, Apr 29 '13 at 22:29

score 10 · Accepted Answer · answered Jul 22 '12 at 01:39

No. In all honesty, if you are not preparing your statements, you are ASKING for a world of hurt.

Just because you escape your quotes with quotes, you are not protecting yourself. Think about this:

A user send you: username'; drop database foo;

You will escape it as username''; drop database foo;

But! if the user does: username\'; drop database foo;

You will be in trouble. You will resolve this for username\''; drop database foo;

Meaning the quote the user placed is escaped, and your quote ended the field username. The drop will then be execute. This is very unsecure.

You need to make sure you Prepare your statements or apply a quote command such as PDO::quote or mysqli_real_escape_string as these escape special characters.

thanks, this showed me that I was not even close to 'thinking outside the box' — tomfumb, Jul 22 '12 at 01:54
Yeah, everything when it comes to security needs to be looked at by at least 6 perspectives. If you don't look at it from the top, front, bottom, left, right and rear, you're not seeing even a 6th of the vulnerabilities. — Mike Mackintosh, Jul 22 '12 at 01:56

score 1 · Answer 2 · answered Jul 22 '12 at 01:07

You've got two options - Escaping the special characters in your unsafe_variable, or using a parameterized query.

$safe_variable = mysql_real_escape_string($_POST["user-input"]);


$mysqli = new mysqli("server", "username", "password", "database_name");    
$unsafe_variable = $_POST["user-input"];
$stmt = $mysqli->prepare("INSERT INTO table (column) VALUES (?)");

Both would protect you from SQL Injection. The parameterized query is considered better practice, but escaping characters in your variable will require fewer changes.

I highly recommend the later. Preparing your statement will save you from a world of hurt. — Mike Mackintosh, Jul 22 '12 at 01:35

score 0 · Answer 3 · answered Jul 22 '12 at 01:27

To answer your question: no, simply removing quotes is not enough to prevent SQL injection.

For example,

"SELECT * FROM user_table WHERE userid=$userid AND password=$password"

Could go badly wrong if $password contained "garbage OR 1", or something like that. So if they've got badly formatted queries, removing quotes won't help.

score 0 · Answer 4 · answered Jul 23 '12 at 09:16

It depends on your database server, and its configuration.

Doubling ' to '' is the ANSI standard way of escaping string literal content, and should be enough.

But, if you're using MySQL or versions of PostgreSQL before 9.1, string literal syntax is not ANSI-conformant by default (it can be, if reconfigured), and the backslash character has a special meaning. This permits attacks like the \' in 680's answer.

Additionally, if you're passing byte strings to your data access function, and your database happens to be using an East Asian character encoding, you'll have the problem that str_replace does a byte replace on a ' that may be part of a multi-byte sequence, ending up with a high byte followed by '': the first quote gets eaten as part of the multi-byte sequence, then the second closes the string. (This doesn't happen in UTF-8 which is designed to avoid trailing low bytes.)

Using the appropriate escaping call for the DB you are using (eg mysql_real_escape_string), or, better, using parameterised queries, means you don't have to know about these obscure and annoying edge cases.

Is this use of str_replace sufficient to prevent SQL injection attacks?

4 Answers4