Passing user input to orderBy is recipe for SQL injection

Today, Laravel's renowned package factory Spatie announced a security fix to their laravel-query-builder package. To clarify the subject, Freek Van der Herten wrote about the issue in his blog, which can be found here: "An important security release for laravel-query-builder"

You're not safe yet

You'd be wrong to think this issue is limited to Spatie's package and you are safe because you don't use it. After reading Freek's post, it becomes clear that there's danger for everyone who uses Laravel's query builder and passes unfiltered user input as column names.

The most common scenario would be building a dynamic order by. Let's consider the following code:

public function index()
{
    $column = request('orderBy', 'title');
    $books = Book::orderBy($column, 'asc')->get();

    return view('books')->with([
        'books' => $books
    ]);
}

Query builder won't parametrize column names

Laravel's SQL injection protection depends on query parametrization. So when passing values to where clauses, Laravel will leverage that feature for you behind the scenes. However, column names cannot be parametrized. There is no such functionality in MySQL. It's therefore wrong to assume that any query that you compose using Laravel is innately safe! Because Laravel won't perform any additional filtering or sanitization of the column names that you pass into the query builder.

An attack might look like this

Back to our Book example. We introduced an easy way of filtering books by any arbitrary column in the table. In doing so, we became vulnerable to SQL injection. How? Like this:

/?orderBy=title->"%27))%23injectedSQL

This translates into the following query:

select * from `books`order by json_unquote(
json_extract(`title`, '$.""')
)#injectedSQL"')) asc

Never trust user input as column names

If you do group by, order by or any other dynamic query building, be sure to whitelist the column names that can be passed as parameters. Even without the SQL injection, attackers can actually discover your table schema or run slow queries on purpose.

Here's another, safer approach to our Books problem:

public function index()
{
    // Use a white-list approach
    $sortableColumns = [
        'title',
        'author',
        'published_year',
    ];

    $column = array_get($sortableColumns, request('orderBy'), 'title');
    $books = Book::orderBy($column, 'asc')->get();

    return view('books')->with([
        'books' => $books
    ]);
}

That's all!

Fire up your editor and do a global search for orderBy, groupBy and select calls across your Laravel codebases. Make sure you never pass user input as column names. Also do visit Freek Van der Herten's original blog post about the issue.

Happy coding!