How to prevent SQL Injection via the array parameter? (CVE-2017-14069)

Hello, this page: https://raw.githubusercontent.com/zhangyuhang2017/cve/master/1.txt suggest that the sql_query

$r = sql_query("SELECT modcomment FROM users WHERE id IN (" . implode(", ", $_POST[usernw]) . ")")or sqlerr(__FILE__, __LINE__);

is vulnerable to a SQL injection “via the usernw array parameter to nowarn.php.”

and the exploit is suggested:

POST nowarned=nowarned&usernw[]=(select*from(select sleep(10))x)

Please kindly how that sql_query should look like so it prevent the abuse?

The problem is allowing user supplied data into the query. The code needs to use Prepared Statements.

I have found one person would do it like this, turn:

$r = sql_query("SELECT modcomment FROM users WHERE id IN (" . implode(", ", $_POST[usernw]) . ")")or sqlerr(__FILE__, __LINE__);

into:

$r = sql_query("SELECT modcomment FROM users WHERE id IN (" . sqlesc(implode(", ", $_POST[usernw]) ). ")")or sqlerr(__FILE__, __LINE__);

that seems a simple fix. I was unable to find any information about sqlesc function.

You can use bind_param for those imploded data into the sql, as this prevent sql injection.
Prepared statement with bind_param that’s what you need do

Your code should look like this


$sql = $sql_query($sq_query->prepare("SELECT modcomment FROM users WHERE id IN (?, ?, ?, ?, ?)"));
$sql->bind_param('sssss', $value1, $value2, $value3, $value4, $value5);
$sql->execute();

I know what you will be thinking now, how do you know how many data is in the array as to figure out how many ??? You are going to bind it with.

But that’s a huge question though

Many people using IN() for unknown number of array data usually ensure is an interger so they can sanitize or esc it rather but the sure way is to bind it.
So push for the question how to bind array data whose number of data it contain is unknown to you or dynamic.

Especially in a select and delete number of selected usernames from database

Take note: it should be ?)"));
I edited it twice but is not updating

I will be here to watch how such can be binded using prepared, i really love to know

Why use bind:

But it tends to become more complicated and thus error prone the more complex a statement gets.

And if there’s just one parameter missing proper processing or handling, the statement is at stake being vulnerable to SQL injection. And to be honest, there are actually many cases here on Stack Overflow which show that this manual technique is more error prone as it’s easy to miss one of the aforementioned points.

In opposite to that, PDO provides a layer of abstraction by having just placeholders in the statement. The parameter values are passed separately and PDO takes care of proper processing and handling. All the developer has to do is prepare the statement with the placeholders and then execute the prepared statement with the actual values.

However, if you would use the value outside a string literal, there would be no need for an attacker to escape from the string literal to inject arbitrary SQL code

It’s in the functions.php file (your programming editor should allow you to perform a search through all the files in a project) and all it is doing is escaping non-numerical values, using the default character set that php is using (utf8.) The problem with this is if that doesn’t match the character set of your database table(s), sql injection is still possible. The only fool proof way of preventing sql special characters in data from breaking the sql query syntax, for all data types and all character encoding, is to use a true prepared query.

As mentioned in your last thread for this TorrentTrader script, it is way out of date (it won’t run at all on current php versions, since the mysql_ extension has been removed from php) and it is filled with security holes, such as the one you are dealing with in this current thread. In searching the web for information about what you are doing in this thread, I found numerous reports of problems, one detailed being able to update any email address, letting someone take over an administrator account. The php code was changed to address this security hole, but the version number wasn’t updated to indicate any type of change was made.

Unless your intent is to actually go through all the code and update the database extension it is using and to update all the queries to be true prepared queries, all you are doing now is wasting your time trying to make an obsolete and insecure script work.

1 Like