How can whitelisting leave dynamic queries vulnerable to SQL injection?

Question

There are quite a lot of various "Is X sufficient to prevent SQL injection?" questions, but I haven't been able to find one that explains this specific thing very well.

Since I can't bind values for column names in prepared statements, there has to be some other safe way to accept user input for ordering queries and other types of things where prepared statements won't help. If I have a $sort_column that I've pulled out of $_GET, I obviously shouldn't just use it directly in the query.

$no = "SELECT col1, col2, col3 FROM table ORDER BY $sort_column;";

Rather than just escaping $sort_column and trying to run the query, it seems like I could check it against a list of legitimate column names for the table in question beforehand. This would let me tell the user they had selected an invalid column without needing to figure it out from a database error. From what I've read, even if I do this, using $sort_column without escaping it is still unsafe, but I don't understand how, unless in_array doesn't work like I think it does.

$good_columns = array("col1", "col2", "col3");
if (in_array($sort_column, $good_columns, true)) {
    $sql = "SELECT col1, col2, col3 FROM table ORDER BY $sort_column;";

And what if I use the column name from my "safe" array rather than using the user supplied value at all?

function is_column($column, $good_columns) {
    $key = array_search($column, $good_columns);
    if ($key !== false) {
        return $good_columns[$key];
    }
    return false;
}

$good_columns = array('col1', 'col2', 'col3');
$my_column = is_column($sort_column, $good_columns);
if ($my_column) {
    $sql = "SELECT col1, col2 FROM tbl1 ORDER BY $my_column;";

I do escape everything, I'm not trying to avoid doing it or advocate not doing it. I'm just curious how something like this would still be vulnerable.

Have a look at http://stackoverflow.com/q/19083323/ and http://stackoverflow.com/q/2683576/ - There are a few examples using PDO prepared statements and `:` placeholders. It should give you some ideas. — Funk Forty Niner, Aug 05 '14 at 01:37

score 1 · Accepted Answer · answered Aug 05 '14 at 01:42

1

The way you do it is not unsafe and you can certainly do it like this. The problem is rather that you might get used to verifying your variables before and then directly inserting them into your query in the process of adding more code to your project. If you miss out on checking a variable at some place in your code it won't be easy for you to find the error or track where an sql injection came from. Better get used to always escape every variable just before you use it in a query.

answered Aug 05 '14 at 01:42

Ke Vin

2,004
1
18
28

"You could forget" -- it's good point. But, for example, i am using prepared statements. Do i need escape everything also? – sectus Aug 05 '14 at 02:11
No, prepared statements do that for you. See this: http://stackoverflow.com/q/8263371/1319254 – Ke Vin Aug 05 '14 at 02:35
1

Prepared statements do not work with table, columns, offset definitinos. So, i need to use white lists and type conversion. – sectus Aug 05 '14 at 02:48

How can whitelisting leave dynamic queries vulnerable to SQL injection?

1 Answers1