ReviewOS

stacks/bun-query-builder

security: SQL injection in identifier + operator interpolation (Stacks audit)

#1009
Closed glennmichael123 opened this 22 days ago · 0 comments
22 days ago

Cross-filed from a Stacks-side security audit (stacksjs/stacks#1858) — the bun-query-builder runtime has several SQL-injection vectors when identifiers or operators come from user input. The Stacks framework has shipped TypeScript surface tightening + identifier/operator validators as defense-in-depth (stacksjs/stacks@b2b587d), but the underlying construction paths need to be fixed here.

Reachability: any consumer that passes a column name or operator from req.body / req.query to the methods below produces user-controlled SQL.

Findings

IDFile:line (dist)Issue
Q-1dist/src/index.js:7507, 7528, 7556whereHas / has / whereDoesntHave quote string values with naked '…' and interpolate operator raw.
Q-2dist/src/index.js:19494, 19470, 19550, 19567ModelQueryBuilder.increment / decrement / pluck / aggregate / count interpolate column + _definition.table raw.
Q-3dist/src/index.js:7625, 8377, 8707, 8738, 8743whereRaw / selectRaw / groupByRaw / havingRaw / orderByRaw accept any string.
Q-4dist/src/index.js:8383, 8389whereColumn / orWhereColumn interpolate left, right, and operator raw.
Q-5dist/src/index.js:8243-8256whereJsonPath interpolates path and op raw on every dialect.
Q-6dist/src/index.js:8154, 8186, 8454, 8514where(string, op, value) parameterises the value but interpolates column + operator raw. Object form where(req.body) exposes column injection.
Q-7dist/src/index.js:9496SQLite identifier quoting is a no-op ((id) => id). SQLite is the typical default driver.
Q-8dist/src/index.js:7306-7308applyCondition is / is not cases interpolate val directly.
Q-9dist/src/index.js:8210-8220whereNull / whereNotNull interpolate column raw.
Q-10dist/src/index.js:8573-8606orderBy / orderByDesc / latest / oldest / reorder / inRandomOrder interpolate column raw.
  • Identifier paths: route through validateIdentifier (already defined internally) or accept only model-attribute keys; reject anything else.
  • Operator paths: constrain at runtime to the documented WhereOperator union.
  • *Raw methods: change the public TS signatures to accept only a RawBuilder<T> tagged-template fragment, never a bare string. Stacks shipped this on its ORM-side facade in stacksjs/stacks@b2b587dwhereRaw: (raw: RawBuilder<unknown>) => M.
  • SQLite quoting: emit "-quoted identifiers with internal-quote doubling (id.replace(/"/g, '""')) — same shape as the Postgres branch.

INVESTIGATE

  • The bundled runtime uses sql([subquerySQL]) (e.g. dist/src/index.js:7994, 8029, 8062, 8095) to re-flag previously-built raw strings as a Bun.SQL fragment. Confirm Bun.SQL's behaviour on array contents — if it's tightened to escape values in the array form, every whereHas/has/doesntHave path breaks.

Filed upstream so the dist code gets the real fix.

Sign in to comment on this issue.