CODEWITHSUNDEEP
phpsqlsecuritysql-injection
tomfumb
tomfumb

Reputation: 3759

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

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.

Upvotes: 5

Views: 8971

Answers (4)

bobince
bobince

Reputation: 536399

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.

Upvotes: 0

Mike Mackintosh
Mike Mackintosh

Reputation: 14237

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.

Upvotes: 10

Rhys
Rhys

Reputation: 1491

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.

Upvotes: 0

Moyed Ansari
Moyed Ansari

Reputation: 8461

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.

Upvotes: 1

Related Questions