SQL injection via the sort field in the ILIAS repository Trash

What a grep cannot find

Anyone working through a large codebase systematically keeps records. In my case that means: for every bug class, the notes say when it was examined, how, and with what result. That is not bureaucracy but the precondition for ever finishing at all – without that list you go round in circles forever.

On 15 July 2026 I had closed one of those classes: SQL injection via a value that a table view takes from the request. The reasoning included this sentence:

No table concatenates the sort field into SQL unescaped – searching for ORDER BY together with the call that supplies the sort field returns zero hits.

The sentence is true. The search really does return zero hits. And it is worthless nonetheless, because it cannot find this shape at all.

The reason is simple once you have seen it: a text search finds things that sit together on one line – that is, the case where someone writes the sort field into the query right where they pick it up. Here it was different. The table picks up the value and passes it as a parameter to a method in another file. Source and sink never appear together on one line; they are not even in the same directory.

I wrote the lesson down as a rule of my own, and it is the real gain from this finding: the set of places to examine derives from the type, not from a search pattern. There is a finite number of table classes, and each of them has exactly one call with which it picks up its sort field. Those calls can be counted. Then you follow each one forward until you know where the value ends up. That is more work than a grep. But it is the only version that supports a conclusion.

The second pass

Twelve days later, on 27 July 2026, the class came up again for a different reason. I was working through the components that had so far only been looked at superficially – this time not hunting for a particular place, but with the blunt question: where is a SQL statement assembled from strings here at all?

That is a laborious way to work and boring for long stretches. Most of what you find that way resolves itself after two glances: the value visibly runs through an escaping function, is converted to an integer, or does not come from outside at all. For the rest there is no shortcut. There the question for every single hit is: where does this value come from, and who determines it?

In one place that question did not end after two steps at a value the program had set itself. The trail ran on and on – all the way to the address bar. It was the repository’s Trash query. And because its value came from a table view, this also reopened the class I had closed twelve days earlier.

The dead ends

In hindsight a finding like this always reads like a straight line. It is not. The same pass includes several places that looked just the same at first and held up on closer reading. One checks the value against the permitted entries, the next maps it explicitly onto the real column name, the third treats the sort direction as an enumeration type. Different routes, all sound – and each of those places showed me what the solution I was actually looking for looks like.

That is exactly what makes such dead ends valuable: they sharpen your picture of what you are actually looking for. Once you have read three sound variants, you recognise the fourth place faster – and above all, you recognise when one is missing.

The following day I then did what I should have done twelve days earlier: counted the places where a sort field is picked up from a table anywhere in the project, and followed each one individually to its end. The Trash remained the only case in which the value arrives in a query unchecked. This time the conclusion holds – not because a search found nothing, but because the list has been worked through.

The path of the value, link by link

Tables in ILIAS sort when you click a column heading. Which column is meant is then held in the address – the table framework stores sort field, sort direction and offset together in one parameter. That is entirely ordinary and has worked for years.

The framework splits this parameter and takes the first part as the sort field:

$nav = explode(":", $this->nav_value);

// $nav[0] is order by
$req_order_field = $nav[0] ?? "";
$req_order_dir   = $nav[1] ?? "";
$req_offset      = (int) ($nav[2] ?? 0);
$this->setOrderField(($req_order_field != "") ? $req_order_field : $this->getDefaultOrderField());
$this->setOrderDirection(($req_order_dir != "") ? $req_order_dir : $this->getDefaultOrderDirection());

The offset is converted to an integer – somebody thought about that one. The direction is normalised further down to asc or desc, with anything else falling back to asc; so nothing could be smuggled in through the direction. None of that happens to the field. It is taken as it arrives.

On its way there the value passes through the application’s general input handling. That sounds reassuring, but it is not here: it strips HTML markup – and nothing else. Whatever carries meaning in a SQL expression survives untouched. For a value that is later processed as a SQL expression, a filter against HTML is about as useful as an umbrella against a flood.

All that is needed now is a view that really does pass this field through to the database. Most do not: they fetch the data and sort it in PHP. The Trash table is one of the others – it explicitly specifies that the database should sort, and passes the field on as one of several parameters to a method that lives in another file.

This transition is exactly where my search of 15 July failed: here the value is still called getOrderField(), there it is only called $order_field – and that is also where the vulnerable line sits:

$order = ' ';
if ($order_field) {
    $order = 'ORDER BY ' . $order_field . ' ' . $order_direction;
}

$query = $select . $from . $this->appendTrashNodeForContainerQueryFilter($filter) . $order;
// ...
$res = $this->db->query($query);

What is notable is what the same query gets right. The Trash table’s filters – search term, deleting user, time range – all run through the database layer’s escaping functions, and the filter for the deleting user is resolved to a user ID beforehand. The difference from the sort clause is not a difference in care but one in kind: filters are values, and for values the standard remedy exists. For a sort field it does not – and that is precisely why this is the spot where something like this happens.

And who reaches this screen?

A sink is only worth as much as the access to it. Here the access was the surprising part: the Trash command checks a single condition – write permission on the container – and nothing else. Anyone who administers a course, group, category or folder anywhere holds that permission. No administrative access, no special role.

For operators, one insight matters most here, and it is the uncomfortable one: exposure depends on the version, not on the configuration. The screen is not only reachable via the visible Trash tab – the corresponding command is accepted by the container interface’s general command dispatch and gates itself on write permission alone. A Trash switched off instance-wide, or an empty one, changes nothing about that either. So there is no setting to turn here; only the update helps.

The list that was already there

Up to this point it is an ordinary story: a value from the request, a missing check, a query. It becomes interesting at the point where you look at what the framework already has within reach at that very spot.

Because the check that is missing here is not hard to build – and it does not need building. It already exists. Every table registers its columns as it is assembled, and in doing so states for each one whether it may be sorted by. The framework keeps a list of that. So at the moment the value arrives from the request, it has the complete enumeration of what would be permissible within reach.

It simply does not consult it. The comparison against that list appears only in the branch that restores a previously stored sort order from the session. And even there it only decides which sort direction is used. If the value arrives fresh from the request, that branch is skipped. The list is left untouched.

A check that hangs off the wrong branch is not half a check. It is none – and in the code it looks like one.

This is the point at which I understood why this spot survived so long. Anyone reading the file finds a comparison against the permitted columns there. It looks right, it is right – just not on the path by which values actually arrive from outside. An existing but misplaced check disguises a missing one better than any absence does. A file without any check at all makes you suspicious. This one you tick off.

From reading to writing

A finding in the source code is a hypothesis. It only becomes solid once it runs – and with this finding, the entire severity rating hung on the question “does it run, and how far?”

What the clause actually permits

You might assume a sort clause is a tightly bounded place: a column name goes there, nothing more. Under MySQL and MariaDB that is wrong. Full expressions are permitted in that clause, including subqueries. That makes the clause not a special case with limited options but full read access to the entire dataset – password hashes, personal data, session data, stored interface secrets. An instance not displaying database errors does not help here; access of this kind does not depend on them.

Getting there involved taking a few peculiarities of this parameter into account – nothing fundamental, but enough for an evening’s work. Those do not belong in this text.

The jump: a default below the application

And then a circumstance comes into play that has nothing to do with the Trash, nor with ILIAS, but changes the rating of this finding more than anything else: the PHP database interface in use permits several statements per call in its default configuration.

This default sits one level below the application code, and it is rarely chosen deliberately – applications generally inherit whatever the interface brings with it. Where it stands open, however, a SQL injection does not stop at reading: whatever additionally comes to be executed may do everything the application’s database user may do – and that user naturally may write. This holds for every PHP application with this default, regardless of where its vulnerable spot sits. It can be switched off, and where the operating environment allows it, that is a worthwhile general hardening measure – independently of this finding.

Confirmation

That a default stands open in theory is a statement about source code. Whether a second statement is actually executed is a statement about a running instance, and only an attempt answers that. I checked it on the instances set up for the purpose, by hand, once per release state.

The result was unambiguous, and it was the same on all four branches maintained at the time – the three release lines and the development state of the following major version: the second statement is executed. I demonstrated this with the most harmless change that makes the point – renaming an account, a field that can be reverted with a single line. The aim was not to get far; the aim was to be able to substantiate the difference between reading and writing. Afterwards it was reverted.

A side effect that prolongs the impact

One last property, which I only noticed while cleaning up: the table framework remembers the sort order last chosen in the acting account’s table settings. That is a convenience feature – you are meant to find the view again the way you left it.

For this finding that means: an injected sort field is not executed once but again on every subsequent visit to that screen – until it is replaced by a valid sort order. So anyone checking after the fact whether something happened on an instance should look not only at the access logs but also at the stored table settings. A trace may have been left behind there that is still running.

The fix

Fifteen days after the report the patch was available. The vendor addressed the sink: the sort field is now escaped as an identifier, and the direction is normalised on top of that.

$valid_direction = strtolower($order_direction) === 'desc' ? 'DESC' : 'ASC';
$order = 'ORDER BY ' . $this->db->quoteIdentifier($order_field) . ' ' . $valid_direction;

That closes this spot reliably, and it is a good fix: it explicitly turns the value into an identifier, and an identifier cannot be a subquery. Anyone concerned only with this one finding is done here.

In the report I had additionally suggested something else – not instead of the fix at the sink, but ahead of it. Namely what the framework could do with the list it already keeps:

$req_order_field = $nav[0] ?? "";
if ($req_order_field !== "" && !in_array($req_order_field, $this->sortable_fields, true)) {
    $req_order_field = "";           // falls back to the default field
}
$this->setOrderField(($req_order_field != "") ? $req_order_field : $this->getDefaultOrderField());

Four lines, no new data structure, no change to an interface: the comparison uses exactly the list that every table fills as it is assembled anyway. The appeal is not its brevity but its reach – it takes effect in one go for every view that sorts in the database, including those that will only exist tomorrow. The fix at the sink takes effect for one query.

This part was not adopted, and I find that understandable: an intervention in the shared table framework touches every table view in the application, and if some column were no longer sortable as a result, that would only surface in production. That is a change with considerable testing effort, and how it should be prioritised against other tasks is something the project can judge better than I can.

Conclusion / outlook

The finding itself is unspectacular: a value from the request, a check that does not cover this path, a concatenation. You find things like that if you search long enough. What I took away from this case is not in the code but in the twelve days before it.

• A search result of zero is not a statement about the code. It is a statement about the search. A text search finds adjacency on one line; a data flow does not respect lines, nor files. Anyone wanting to close a bug class has to count the places to examine – from the type, not from a pattern – and follow each one through to the end. Anything else merely records your own confidence.
• A misplaced check disguises better than no check. The comparison against the permitted columns was there, it was correct, and it hung off the wrong branch. Code without any check makes you suspicious; code with a check in the wrong place reads as settled. So the question is never “is there a check?” but always “is there a check on this path?”
• Sort clauses are a structural blind spot. Not in ILIAS – everywhere. They are the place where the standard answer to SQL injection does not apply in principle, because a placeholder cannot replace an identifier. Every application with sortable tables has to come up with something of its own there, and each does it a little differently. If you want to find something in an application of your own within the hour: start here.

This finding was the beginning. The question it grew out of – where is a value passed along without anyone checking it on the way? – can also be asked quite differently: not about values that travel into a query, but about stored state that becomes an object again. A few days later that very variant led to a finding that did not even require an account: the unauthenticated PHP object injection in the Shibboleth logout endpoint.