Skip to main content
← All articles
craft cms

Craft quietly ignored my eager-loaded orderBy — and the reason is a security boundary

Craft quietly ignored my eager-loaded orderBy — and the reason is a security boundary

A query returned the wrong row and didn't say a word about it. No error, no warning — an eager-loaded Matrix relation with orderBy and limit: 1 just handed back the first block instead of the one I asked for. I went digging, and the answer turned out to be more interesting than a bug: Craft's eager loading was quietly dropping my sort on purpose, because that same code path had been a SQL-injection hole twice this autumn.

The bug

Issue #19992, opened Oct 15 by denisyilmaz on Craft 5.11.5: GraphQL orderBy is silently ignored on eager-loaded fields. The repro was a Matrix field on a Commerce product; orderBy did nothing, and limit: 1 returned the first block in natural order rather than the intended one. (inReverse and limit on their own still worked.) The reporter traced it into Elements::_eagerLoadElementsInternal(), where a call to cleanseQueryCriteria() was unsetting orderBy entirely. Sort from a root entries query worked fine; only the eager-loaded path ate it. Brandon Kelly self-assigned it the same day.

The fix, and the tell

The fix — PR #19994, merged Oct 16 — is where it gets instructive. I read the diff. Instead of restoring orderBy blindly, it keeps your sort only if it passes a new guard, Db::isSafeOrderBy() (tagged @since 5.11.6). "Safe" means an ExpressionInterface, comma-separated column names (optionally table-prefixed, optionally asc/desc), or an array mapping columns to SORT_ASC/SORT_DESC. Anything else gets dropped and now logs a warning ("Ignoring unsafe orderBy criteria for the eager-loaded … elements") instead of vanishing in silence. Raw SQL like field(id, 3, 1, 2) now has to be passed explicitly as an expression. The PR also teaches eagerly() to pass custom orderBy, offset and limit through, and fixes a separate double-reversal bug when orderBy meets inReverse.

That shape — an allowlist of "safe" sort forms, with everything else refused — is the fingerprint of a security fix, not a feature. And the history backs it up. Two High-severity advisories from this autumn, both patched in 5.10.13 (Aug 4) and published Sep 1, live in this exact code:

  • GHSA-4mgp-5vf2-7c9m (High, CWE-89): authenticated SQL injection via nested eager-loading criteria. The precondition is only accessCp plus a Team/Pro edition, through the element-index endpoint.
  • GHSA-9c4j-cjw3-r3xx (High, CWE-470): low-privilege RCE through element-search eager loading — criteria.with[*].when survived input cleansing and was later invoked as a PHP callback. Affected >=5.8.0 <5.10.13.
Eager loading is sold as a performance feature, but it's also the seam where client-supplied query criteria reach the database. Dropping orderBy looks like the cheap way to stop injection through that seam — so "my sort got ignored" and "SQL injection" are the same story told from different ends.

I should be clear that the security-boundary read is my inference from the issue and the diff. The PR description doesn't call #19994 a security fix, and it doesn't formally link the dropped sort to the earlier SQLi — both just happen to pass through the same criteria-cleansing code. Take the connection as a developer's hunch, not a vendor statement.

What a Drupal hand notices

In Drupal, a field's delta order is stable and loadMultiple() doesn't take user-supplied query criteria — so this whole category of surprise doesn't really exist there. Craft, by contrast, hands query-builder criteria straight to Twig and GraphQL clients, which is more powerful and lands the trade-off differently: the flexibility is exactly what makes the sort an attack surface. That's my instinct talking; I'm not going to assert more about Drupal's loaders than I've actually leaned on.

If you're on 5.11.5 right now

The fix is on the 5.x branch but, as of Oct 19, unreleased — the latest tag I see is 5.11.5.1 from Oct 8, and the diff's @since 5.11.6 is the only version evidence. So this isn't "update and move on" yet. If you have an eager-loaded Matrix query with orderBy plus a tight limit, test it — it can return the wrong element with no error at all. And don't take a workaround from me on faith: whatever you switch to (sorting after load, or a root query with ownerId), verify it returns what you expect before you ship it. This bite was narrow — a Matrix field via GraphQL on Commerce — so don't assume every query is affected either.

Links

BM
Blue Moose
The moose behind Blue Moose. Full-stack PHP developer — Drupal by day, Symfony by night, tests always.