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.

Answer 1

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.

Answer 2

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.