is discussed. The section starts with the following statement:

SQL injection is one of the oldest attacks against web 
applications. However, as it is well known, there are 
many techniques to defend against it

I fully agree with this sentence. I even agree with the list of methods that follows, although I cannot accept, that prepared statements are considered more secure than the use of mysql_real_escape_string(). If both techniques are used correctly they are both secure. (Unless there is a security vulnerability in their implementation).

So far so good... The section continues with some statements or personal opinions that you can agree or disagree... The article gets interesting however when OWASP describes how to defend against SQL Injection. They present two examples, one using prepared statements and one using just escaping and number conversion functions.

When I read it the first time, I was not prepared for the shock I got. I have seen a lot of very bad code during my years of auditing but I have never seen such a scary security hole in security teaching material before. (Well not true, I have disclosed similiar scary holes in the advertised sources of this article...)

Let's start with the SQL statement they wanted to protect. (The point of origin is not mentioned in the article).

$sql = "SELECT  * FROM ".
	$_POST['DBHOST_TBL_PREFIX'].
	"themes WHERE themeid='$themeid'";
mysql_query($sql, $conn);

If you are unfamiliar with SQL Injection all you need to know here is that manipulation of the parameter $themeid or by changing the POST variable DBHOST_TBL_PREFIX it is possible to influence what SQL query is executed on the server. This can lead in this example to information disclosure. (Actually, beside the fact that it is a stupid idea to give the user the chance to influence the tablename at all, some SQL purists will also criticize the use of * instead of listing the required fields)

Now I will present the offical OWASP solutions to this problem to you. You will find both examples that follow in their PHP Top 5 List. Well until they get removed. Do not use them, they are dangerous.

Solution 1 - Escaping/Number Conversion

$sql = "SELECT  * FROM “. 
	mysql_real_escape_string($_POST['DBHOST_TBL_PREFIX']).
	”themes WHERE themeid='”.intval($themeid)."'";

mysql_query($sql, $conn);

Solution 2 - Using prepared statements

$sql = "SELECT  * FROM “. 
$mysqli->escape_string($_POST['DBHOST_TBL_PREFIX']).
”themes WHERE themeid=?”;

$statement = $mysqli->prepare($sql);
$statement->bind_param(’i’, $themeid);
$statement->execute();

The educated eye will see, that $themeid is explicitly converted to a number in the first solution and bound as integer parameter to the prepared statement in the second solution. Additionally one can see, that the author tries to handle the more dangerous SQL Injection through the user supplied table prefix by escaping it with the database escaping function. Unfortunately that is based on a huge misunderstanding of the database escaping function. The database escaping functions are meant to escape data-strings in SQL queries. They are NOT meant for escaping f.e. SQL keywords, database or table names. And even if they were, all string escaping in SQL queries are worthless if the escaped data is not enclosed in apostroph characters (').

There is no function to convert user supplied data to valid table names. The best solution (if you really want user supplied tablenames... you dont want this, do you?) for the problem above would be a whitelist approach and only allow a restricted charset.

Corrected Solution

$sql = "SELECT  * FROM “.
preg_replace("/[^A-Za-z0-9_]/", "", $_POST['DBHOST_TBL_PREFIX']).
”themes WHERE themeid=?”;

$statement = $mysqli->prepare($sql);
$statement->bind_param(’i’, $themeid);
$statement->execute();

The corrected solution is now SQL injection safe up to the degree that was intended by the query designer. Please keep in mind, that I strongly disagree with giving the user the right to choose what table prefix he wants through a POST variable. Depending on the database design, this could result in further information disclosure if it is possible for the attacker to specify a tablename he should not have access to. Additionally it is trivial to supply a name to a nonexisting table. This could trigger an SQL error, that depending on the configuration is printed into the output.

Finally I want to thank you for reading through this demonstration of blatant holes in security teaching material that is advertised as best practice by some people. Maybe this helps you to understand, why I strongly disagree with some of these people. If you have any further questions, then don't hesitate to contact me.