This is an archived copy of the forum for reference purposes

SQL Injection in Non-Value Field

posted by justbn 8 years ago

I've got a problem where I need to "escape" user input for a part of a query that is NOT a value. Here's the scenario:

I'm using jqGrid to display many tables with dozens of different column names. If a user sorts or filters and column, I get the request via AJAX. I then know what columns to order on.

I've developed one master query that will query the appropriate table, the right columns, group, order by, limit etc. This way, I don't need a query for each table.

However, the problem is with the risk of SQL injection on the order by or group by clause as the column names are provided by the user. I searched a bit and found a few discussions on this, but they all seem to say the same thing : use white lists of column names to compare against. That is probably my only solution, but it is not very friendly to future changes.

Do you have any suggestions for this within flourish? Of course, you can't escape these because that would put quotes around them and produce invalid SQL. Ideas?

FYI : http://josephkeeler.com/2009/05/php-security-sql-injection-in-order-by/

Also, I am using raw SQL queries. I can imagine that fORM could prevent this as it knows what all columns are actually in the db. However, I've got some pretty complex queries that are easiest to do directly with SQL.

I've got a not so wonderful solution. Basically, I check if the sort field is in the list of fields requested in the SQL. If it is not, I throw an exception.

It works but I'm sure there is a better solution out there.


function modifyQueryForGrid($sql)
{
	$sort_order = fRequest::get('sord','string?','asc');
	$sort_field = fRequest::get('sidx','string?',null);
	
	if($sort_field == null)
	{
		return $sql;
	}

	if( $sort_order != 'asc' && $sort_order != 'desc' )
	{
		throw new Exception("Invalid Sort order($sort_order)");
	}
	
	
	// Get the list of requested fields from the sql statement
	$select_end = strpos($sql, 'select') + 7;
	$from_begin = strpos($sql, ' from');
	$length = $from_begin - $select_end;
	$fields = preg_replace('/\\s+/', '', substr($sql, $select_end, $length));
	$fields = explode(',', $fields);
	
	// The sort_field must match a field in the sql statement.
	if (! in_array($sort_field, $fields) ) 
	{
		throw new Exception("Invalid Sort on Column ($sort_field)");
	}
	
	$sql = $sql . " order by $sort_field $sort_order";
	
	return $sql;
}

posted by justbn 8 years ago

You can use the identifier %r placeholder with fDatabase to escape table and column names.

$column    = fRequest::get('column');
$direction = fRequest::getValid('direction', array('asc', 'desc'));

$result = $db->query("SELECT * FROM users ORDER BY %r " . $direction, $column);

The downside to this type of approach, versus white-listing, is that you can easily get an fSQLException if a user enters a column that does not exist. You could always use fSchema to get a list of columns to create a white-list.

posted by wbond 8 years ago

Argh... Thanks for the answer. I'm sorry to have wasted your time. I just didn't see that in the docs. I stopped at 'Escaping Data'.

RTFM!

In the end, I did not use my example above. I ended up creating a function and passing in a whitelist of valid column names in.

posted by justbn 8 years ago

It certainly wasn't a waste of time. Discussions like this are a good resource for other people.

posted by wbond 8 years ago