fix(model): block SQL injection in legacy finders via adapter-side value 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>

Author: bpamiri

vendor/wheels/databaseAdapters/Base.cfc

@@ -472,6 +472,18 @@ component output=false extends="wheels.Global"{
 
 	/**
 	 * Internal function.
+	 *
+	 * For integer/float/boolean columns this returns the value unquoted so the
+	 * downstream WHERE-clause regex can re-extract bare numerics into
+	 * cfqueryparam placeholders. That contract is unsafe by itself — a string
+	 * like "0 OR 1=1" would land verbatim in the SQL — so we validate the value
+	 * shape here before passing it through. This closes SQL injection across
+	 * every caller (chainable QueryBuilder, $keyWhereString used by findByKey/
+	 * updateByKey/deleteByKey, dynamic finders findByX/findOneByX/findAllByX,
+	 * the uniqueness-check $buildWhereClausePart, and any future caller).
+	 *
+	 * String columns are unaffected — the adapter still wraps and escapes the
+	 * value, so classic single-quote payloads land harmlessly inside a literal.
 	 */
 	public string function $quoteValue(required string str, string sqlType = "CF_SQL_VARCHAR", string type) {
 		if (!StructKeyExists(arguments, "type")) {
@@ -480,11 +492,50 @@ component output=false extends="wheels.Global"{
 		if (!ListFindNoCase("integer,float,boolean", arguments.type) || !Len(arguments.str)) {
 			local.rv = "'#Replace(arguments.str, "'", "''", "all")#'";
 		} else {
+			$validateValueShape(arguments.str, arguments.type);
 			local.rv = arguments.str;
 		}
 		return local.rv;
 	}
 
+	/**
+	 * Validate that a value matches the shape this adapter expects for its
+	 * declared column type. Throws Wheels.InvalidValue when the shape doesn't
+	 * match — closing the SQL-injection vector through which strings like
+	 * "0 OR 1=1" would otherwise land in the unquoted numeric/boolean path.
+	 *
+	 * Intentionally narrow: only fires for integer/float/boolean columns,
+	 * which is exactly the set of types $quoteValue passes through unquoted.
+	 * String columns are wrapped + escaped above and don't need this check.
+	 */
+	public void function $validateValueShape(required string str, required string type) {
+		switch (arguments.type) {
+			case "integer":
+				if (!ReFind("^-?[0-9]+$", arguments.str)) {
+					$throwInvalidValue(arguments.str, "integer");
+				}
+				break;
+			case "float":
+				if (!ReFind("^-?[0-9]+(\.[0-9]+)?$", arguments.str)) {
+					$throwInvalidValue(arguments.str, "float");
+				}
+				break;
+			case "boolean":
+				if (!ListFindNoCase("0,1,true,false,yes,no", arguments.str)) {
+					$throwInvalidValue(arguments.str, "boolean");
+				}
+				break;
+		}
+	}
+
+	public void function $throwInvalidValue(required string str, required string expectedType) {
+		Throw(
+			type = "Wheels.InvalidValue",
+			message = "The value `#EncodeForHTML(arguments.str)#` is not a valid #arguments.expectedType#.",
+			extendedInfo = "Values bound to #arguments.expectedType# columns must be valid #arguments.expectedType# literals so they can be safely interpolated into the WHERE clause. This check protects every Wheels query path against SQL injection through typed-numeric/boolean payloads."
+		);
+	}
+
 	/**
 	 * Acquire a database advisory lock with the given name.
 	 * Advisory locks are application-level locks that don't lock rows or tables.

vendor/wheels/tests/specs/security/LegacyFinderInjectionSpec.cfc

@@ -0,0 +1,134 @@
+component extends="wheels.WheelsTest" {
+
+	function run() {
+
+		g = application.wo
+
+		describe("Legacy finder SQL injection prevention", () => {
+
+			// PR #2416 closed the SQL-injection vector in the chainable QueryBuilder by
+			// validating typed values before they reach the adapter's $quoteValue. The
+			// same root-cause pattern existed in the legacy ORM paths that pre-date the
+			// chainable builder — findByKey / updateByKey / deleteByKey (which share the
+			// $keyWhereString sink in sql.cfc) and the dynamic finders (findByX /
+			// findOneByX / findAllByX) in onmissingmethod.cfc. Adapter-side validation
+			// in Base.$quoteValue closes those at the only sink they share.
+			//
+			// Tracking issue: #2417.
+
+			describe("$keyWhereString — findByKey / updateByKey / deleteByKey", () => {
+
+				it("findByKey rejects tautology injection on integer-keyed model", () => {
+					expect(function() {
+						g.model("post").findByKey("1 OR 1=1");
+					}).toThrow("Wheels.InvalidValue");
+				});
+
+				it("findByKey rejects UNION SELECT injection", () => {
+					expect(function() {
+						g.model("post").findByKey("1 UNION SELECT password FROM c_o_r_e_users");
+					}).toThrow("Wheels.InvalidValue");
+				});
+
+				it("findByKey rejects comment-truncation injection", () => {
+					expect(function() {
+						g.model("post").findByKey("1 --");
+					}).toThrow("Wheels.InvalidValue");
+				});
+
+				it("findByKey rejects stacked-statement injection", () => {
+					expect(function() {
+						g.model("post").findByKey("1; DROP TABLE c_o_r_e_posts");
+					}).toThrow("Wheels.InvalidValue");
+				});
+
+				it("updateByKey rejects tautology injection in key", () => {
+					expect(function() {
+						g.model("post").updateByKey(key = "1 OR 1=1", title = "hijacked");
+					}).toThrow("Wheels.InvalidValue");
+				});
+
+				it("deleteByKey rejects tautology injection in key", () => {
+					expect(function() {
+						g.model("post").deleteByKey("1 OR 1=1");
+					}).toThrow("Wheels.InvalidValue");
+				});
+
+				it("findByKey accepts a legitimate integer key", () => {
+					// Should not throw; the row may or may not exist depending on
+					// fixtures, but the value-shape gate must let integers through.
+					expect(function() {
+						g.model("post").findByKey(1);
+					}).notToThrow();
+				});
+
+				it("findByKey accepts an integer-shaped string key", () => {
+					expect(function() {
+						g.model("post").findByKey("1");
+					}).notToThrow();
+				});
+
+				it("findByKey accepts a negative integer key", () => {
+					// Negative integers are legitimate per the regex (^-?[0-9]+$).
+					// No row will match, but the gate must not throw.
+					expect(function() {
+						g.model("post").findByKey("-1");
+					}).notToThrow();
+				});
+
+			});
+
+			describe("dynamic finders — findByX / findOneByX / findAllByX", () => {
+
+				it("findAllByViews rejects tautology injection on integer column", () => {
+					expect(function() {
+						g.model("post").findAllByViews("0 OR 1=1");
+					}).toThrow("Wheels.InvalidValue");
+				});
+
+				it("findOneByViews rejects UNION SELECT injection on integer column", () => {
+					expect(function() {
+						g.model("post").findOneByViews("0 UNION SELECT 1");
+					}).toThrow("Wheels.InvalidValue");
+				});
+
+				it("findOneByViews rejects comment-truncation on integer column", () => {
+					expect(function() {
+						g.model("post").findOneByViews("0 --");
+					}).toThrow("Wheels.InvalidValue");
+				});
+
+				it("findAllByAveragerating rejects injection on float column", () => {
+					expect(function() {
+						g.model("post").findAllByAveragerating("3.14 OR 1=1");
+					}).toThrow("Wheels.InvalidValue");
+				});
+
+				it("findAllByViews accepts legitimate integer value", () => {
+					expect(function() {
+						g.model("post").findAllByViews(0);
+					}).notToThrow();
+				});
+
+				it("findAllByAveragerating accepts legitimate float value", () => {
+					expect(function() {
+						g.model("post").findAllByAveragerating(3.14);
+					}).notToThrow();
+				});
+
+				it("findAllByTitle accepts ordinary string values (string column)", () => {
+					// String columns are quoted-and-escaped by the adapter; the
+					// numeric/boolean shape gate is skipped for them so legitimate
+					// string content (including spaces) passes through.
+					expect(function() {
+						g.model("post").findAllByTitle("any title here");
+					}).notToThrow();
+				});
+
+			});
+
+		});
+
+	}
+
+}