3
\$\begingroup\$

I have to process raw sql user input with Laravel. I've backed them up as follows:

$query = DB::table('table_name');

function escapeString($str) {
    return DB::connection()->getPdo()->quote($str);
}

$column = escapeString($rule['query']['operand']);
$value = escapeString($rule['query']['value']);
if(in_array($rule['query']['operator'], $query->operators)) {
    return $column . ' ' . $rule['query']['operator'] . ' ' . $value;
}

Is that enough, or can I still be attacked over it?

I read:

(This question was postet originaly at https://stackoverflow.com/questions/63091979/is-my-code-protected-against-sql-injection, but STA suggest to post this question here again)

Update:

I figured out, how to use value in variable binding. Also I changed escapeString to

$column = preg_replace('/[^a-zA-Z_]/', '', $rule['query']['operand']);

Thats fine for alle columns names and I am pretty sure that this is safe. This filtering approch ist also used in https://stackoverflow.com/questions/10080850/using-a-whitelist-on-user-input

\$\endgroup\$
0

1 Answer 1

2
\$\begingroup\$

The PDO::quote() function is not correct for making column names safe. It's basically a wrapper for the QUOTE() function in MySQL:

The string is returned enclosed by single quotation marks and with each instance of backslash (), single quote ('), ASCII NUL, and Control+Z preceded by a backslash. If the argument is NULL, the return value is the word “NULL” without enclosing single quotation marks.

The enclosing single-quote marks make the result usable as a string value in SQL, but you are using it for a column name. So you would end up with:

'mycolumn' = 'myvalue'

The single quotes make it not act like a column in your expression. The QUOTE() function is for string or date values, not identifiers like column names or table names.

You posted an update, where you changed to using a bound query parameter for the value, and you used a regular expression replacement function to strip out any non-word characters from the column name.

But this still risks the column being a reserved SQL keyword, which will cause errors in your code:

SELECT = 'myvalue'

To avoid this, the column name should be enclosed in backticks:

`SELECT` = 'myvalue'

If you enclose the column in backticks, then the column may be a reserved keyword, or it can even have punctuation, whitespace, international characters. Those are legitimate for SQL identifiers, but your escaping function would not allow it.

You should make sure if the column name contains a literal back-tick, you double it, which will make it act like a literal back-tick in your SQL.

function quoteIdentifier($str) {
    return '`' . preg_replace('/`/', '``', $str) . '`';
}
\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.