Skip to main content
← All articles
craft cms

A September SQL-injection fix in Craft missed a second path — this is the one that stayed open

A September SQL-injection fix in Craft missed a second path — this is the one that stayed open

Most SQL injection writeups describe a single unsanitized parameter reaching a single query. GHSA-cphx-cx67-jcqj, published November 2 (CWE-89, High severity), is more interesting than that: it's a second, distinct path into the same vulnerable sink that a fix shipped a month earlier — for a related advisory — didn't close.

How a criteria array becomes raw SQL

The affected sink is Craft's Element::findOne(), called from several controllers with a user-supplied parameter passed straight into the query's criteria array without being cast to a scalar first. Yii's query builder has a behavior that's easy to forget is there: when a criteria array's value for a given key is itself an array shaped like a raw condition, Yii treats it as literal SQL rather than a value to bind. A nested array crafted as input can set the query's where property directly — and because this injection happens inside the criteria-building step, it executes before any ownership or permission check on the record being queried gets a chance to run. The injection isn't a side door past the permission system; it fires before the permission system is even consulted.

Craft 5.0.0 up to before 5.11.5 is affected; the fix shipped in 5.11.5 on October 7. The advisory identifies four concrete controller actions reachable this way, two of which need no special permission at all: actions/users/delete-address and actions/users/save-address are open to any authenticated user managing their own address book, while actions/assets/preview-thumb and actions/assets/show-in-folder require the accessCp permission. None of the four require anything close to administrator access. And because the injection point sits in a where clause rather than something reflected back in a response body, the practical exploitation path is blind — boolean-based or time-based extraction, the slow but reliable kind that doesn't need an error message to leak data.

The part that should give every security team pause

What makes this advisory worth separating out from an ordinary injection bug is a single sentence in Craft's own writeup: this exact sink had already been the subject of a fix, GHSA-mjj9-pjh4-r48g, shipped in 5.11.2 in September. That fix closed one way of reaching findOne() with attacker-controlled criteria. It did not close this one. The underlying defect — a sink that will execute literal SQL if a criteria array is array-shaped in the right way — was never actually removed; only one of its entry points was. A second entry point into an unpatched sink isn't a new class of bug requiring new understanding; it's the same bug, found again, because the fix addressed a caller instead of the sink itself.

The engineering lesson, not just the security one

This is the argument for fixing at the boundary rather than at the call site whenever a sink has more than one way to reach it. Casting or validating at findOne() itself — refusing any criteria value shaped like a raw condition unless explicitly intended — closes every path at once, known and undiscovered. Patching each caller as it's found means every subsequent audit has to ask "did we get all of them?" and the answer, here, was no. It's the same instinct Drupal's own conventions lean on: prefer the framework's query-building layer to reject the dangerous shape outright, rather than trusting every call site to remember to sanitize before it gets there.

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