fix(model): block SQL injection in chainable QueryBuilder via value-shape validation#2416
Merged
Merged
Conversation
…k SQL injection The chainable QueryBuilder advertised in CLAUDE.md and its own docblock as "injection-safe" relied on the adapter's $quoteValue, which passes integer, float, and boolean values through unquoted. Without a value-shape check at the builder boundary, a string like "0 OR 1=1" landed verbatim in the WHERE clause on integer columns and bypassed the post-hoc cfqueryparam pass — a tautology injection that would return every row. The same defect affected whereBetween (per-bound), whereIn / whereNotIn (per-element), and the 2-arg where()/orWhere(). Validate values against the declared validationtype in $quoteValue. Integer and float values must match a numeric regex; booleans must be one of 0/1/true/false/ yes/no. Mismatches throw Wheels.InvalidValue. String columns are unaffected — the adapter still quotes-and-escapes them. Adds 22 regression tests in QueryBuilderSpec.cfc covering tautology, UNION SELECT, comment-truncation, stacked-statement, and per-method coverage across where, orWhere, whereBetween, whereIn, whereNotIn — plus accept-cases for legitimate integer/float/string values. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bpamiri
added a commit
that referenced
this pull request
May 3, 2026
…lue validation (#2417) (#2418) PR #2416 closed the chainable QueryBuilder's SQL injection by validating typed values before $quoteValue. The same root-cause pattern still existed in the legacy ORM paths that pre-date the chainable builder: - findByKey / updateByKey / deleteByKey via $keyWhereString (sql.cfc:1325) - findByX / findOneByX / findAllByX via dynamic finders (onmissingmethod.cfc:165) - $buildWhereClausePart used by validatesUniquenessOf (validations.cfc:774) All three paths reach the adapter's $quoteValue with user-supplied values bound to integer/float/boolean columns, where the adapter passes them through unquoted. The downstream RESQLWhere regex re-extracts bare numerics into cfqueryparam placeholders but leaves any injected OR / UNION / comment structurally intact — so /users/1%20OR%201%3D1 produced WHERE id=1 OR 1=1, a tautology returning the first row regardless of the intended key. Push the validation into Base.$quoteValue itself so every caller (including any future one) inherits the protection. The same regex/allowlist as #2416 — ^-?[0-9]+$ for integer, ^-?[0-9]+(\.[0-9]+)?$ for float, 0/1/true/false/yes/no for boolean. Mismatches throw Wheels.InvalidValue. String columns are untouched; the adapter still wraps + escapes them. Adapter-side rather than caller-side per the audit notes in #2417: read.cfc:170 round-trips DB-stored PK values (always valid integers); validations.cfc:774 feeds model property values (a non-numeric on an integer column was already a DB error today, now it's a clearer Wheels.InvalidValue at the framework boundary); Base.cfc:66 fires only on parameterize=false query plans where malformed numerics were also already broken. Adds 16 regression tests in vendor/wheels/tests/specs/security/ LegacyFinderInjectionSpec.cfc covering tautology, UNION SELECT, comment- truncation, and stacked-statement payloads against findByKey / updateByKey / deleteByKey / findAllByX / findOneByX on integer + float columns, plus accept-cases for legitimate integer/float/string values. Closes #2417. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Pre-release security review for v4.0.0-rc1 surfaced a HIGH-severity SQL injection in the new chainable
QueryBuilder— the API advertised in CLAUDE.md and its own docblock as "injection-safe."The adapter's
$quoteValuereturns integer/float/boolean values unquoted, on the assumption that callers have already constrained the value to a numeric/boolean shape. The QueryBuilder didn't enforce that constraint. A standard endpoint like:function index() { users = model("User").where("age", ">", params.age).get(); }/users?age=0%20OR%201%3D1producedWHERE age > 0 OR 1=1— a tautology returning every row. The same defect affectedwhereBetween(per-bound),whereIn/whereNotIn(per-element), and the 2-argwhere()/orWhere()forms.What changed
$quoteValuenow calls$validateValueShapebefore delegating to the adapter. Integer values must match^-?[0-9]+$, floats must match^-?[0-9]+(\.[0-9]+)?$, booleans must be in0,1,true,false,yes,no. Mismatches throwWheels.InvalidValue. String columns are unaffected — the adapter still quotes and escapes them.where,orWhere,whereBetween,whereIn,whereNotIn— plus accept-cases for legitimate integer/float/string values.Out of scope (separate finding)
The same root-cause pattern exists in legacy paths that pre-date the chainable builder:
vendor/wheels/model/onmissingmethod.cfc:165— dynamic finders (findByViews(params.views))vendor/wheels/model/sql.cfc:1325—$keyWhereStringused byfindByKey/updateByKey/deleteByKeyThese are vulnerable today (
findByKey("1 OR 1=1")injects on integer-keyed models), but they ship in every Wheels release going back to before 4.0. Recommend tracking as a follow-up so the rc1 fix stays narrowly scoped to the new "injection-safe" surface.Test plan
pr.yml(Lucee 5/6/7 + Adobe 2018/2021/2023/2025 + boxlang × SQLite/MySQL/Postgres/SQL Server)where("views", ">", 0),whereBetween("views", 1, 5),whereIn("lastName", "Djurner,Petruzzi")all match the new validation)QueryBuilderSpec.cfcpass on all enginesNote on local verification: I could not run the suite locally — LuCLI hardcodes shutdown port
8081, which is held by another worktree's daily server. Did not bounce that server. Fix verified by close code review against the test suite.🤖 Generated with Claude Code