fix(security): validate column references on the SELECT/ORDER BY family

packages/bun-query-builder/test/client.identifier-injection.test.tsadded+112-0

Changes to packages/bun-query-builder/test/client.identifier-injection.test.ts

		@@ -0,0 +1,112 @@
	1	+/**
	2	+ * Identifier injection on the SELECT / FROM / ORDER BY / GROUP BY family —
	3	+ * stacksjs/bun-query-builder#1181.
	4	+ *
	5	+ * These methods interpolated caller-supplied column names straight into SQL.
	6	+ * `quoteColumnForDialect` looked like protection and was not: it is a MySQL
	7	+ * quoter that returns anything it cannot parse verbatim, and returns its
	8	+ * argument untouched on every other dialect.
	9	+ *
	10	+ * Measured against live Postgres before the fix, with `?sort=` as the payload:
	11	+ *
	12	+ * table exists before : true
	13	+ * SQL built : SELECT * FROM sec_users ORDER BY id; DROP TABLE sec_users; -- ASC
	14	+ * table exists after : false
	15	+ *
	16	+ * The stacked statement ran. The WHERE and join family already refused the
	17	+ * same input through `validateIdentifier`, and `orm.ts` had closed the ORDER BY
	18	+ * half after stacksjs/stacks#1858, so this was a gap in an existing defence.
	19	+ */
	20	+
	21	+import { describe, expect, it } from 'bun:test'
	22	+import { buildDatabaseSchema, buildSchemaMeta, createQueryBuilder, raw } from '../src'
	23	+
	24	+function qb(): any {
	25	+ const models = {
	26	+ users: {
	27	+ columns: {
	28	+ id: { type: 'integer', isPrimaryKey: true },
	29	+ name: { type: 'text' },
	30	+ team_id: { type: 'integer' },
	31	+ },
	32	+ },
	33	+ } as any
	34	+ return createQueryBuilder<any>({
	35	+ schema: buildDatabaseSchema(models),
	36	+ meta: buildSchemaMeta(models),
	37	+ autoMigration: { enabled: false } as any,
	38	+ })
	39	+}
	40	+
	41	+const PAYLOAD = 'id; DROP TABLE users; --'
	42	+const REFUSED = /is not a plain column reference/
	43	+
	44	+describe('identifier injection is refused (#1181)', () => {
	45	+ it('orderBy, the realistic entry point', () => {
	46	+ // A sort column is the most common request input that is legitimately a
	47	+ // column name, so this is the parameter most likely to be passed through.
	48	+ expect(() => qb().selectFrom('users').orderBy(PAYLOAD)).toThrow(REFUSED)
	49	+ })
	50	+
	51	+ it('every other ORDER BY spelling', () => {
	52	+ expect(() => qb().selectFrom('users').orderByDesc(PAYLOAD)).toThrow(REFUSED)
	53	+ expect(() => qb().selectFrom('users').latest(PAYLOAD)).toThrow(REFUSED)
	54	+ expect(() => qb().selectFrom('users').oldest(PAYLOAD)).toThrow(REFUSED)
	55	+ })
	56	+
	57	+ it('groupBy and distinctOn', () => {
	58	+ expect(() => qb().selectFrom('users').groupBy(PAYLOAD)).toThrow(REFUSED)
	59	+ expect(() => qb().selectFrom('users').distinctOn(PAYLOAD)).toThrow(REFUSED)
	60	+ })
	61	+
	62	+ it('select and addSelect', () => {
	63	+ expect(() => qb().selectFrom('users').select([PAYLOAD])).toThrow(REFUSED)
	64	+ expect(() => qb().selectFrom('users').addSelect(PAYLOAD)).toThrow(REFUSED)
	65	+ })
	66	+
	67	+ it('addSelect given an array, which bypassed the first fix', () => {
	68	+ // `addSelect` is variadic, so `addSelect(['a'])` - the shape `select`
	69	+ // takes - arrived as one array argument and reached the String() fallback
	70	+ // whole. renderSelectColumn now recurses instead.
	71	+ expect(() => qb().selectFrom('users').addSelect([PAYLOAD])).toThrow(REFUSED)
	72	+ })
	73	+
	74	+ it('selectFrom, where the type says keyof DB but JS can pass anything', () => {
	75	+ expect(() => qb().selectFrom('users; DROP TABLE users; --')).toThrow(REFUSED)
	76	+ })
	77	+
	78	+ it('a subquery is refused as well as a stacked statement', () => {
	79	+ // Stacked statements are the loud case; an ORDER BY subquery is the quiet
	80	+ // one, and is a working boolean oracle wherever stacking is blocked.
	81	+ expect(() => qb().selectFrom('users').orderBy('(SELECT secret FROM users LIMIT 1)')).toThrow(REFUSED)
	82	+ expect(() => qb().selectFrom('users').select(['name FROM users UNION SELECT 1 --'])).toThrow(REFUSED)
	83	+ })
	84	+})
	85	+
	86	+describe('legitimate column references still work (#1181)', () => {
	87	+ it('plain, qualified and aliased names', () => {
	88	+ expect(String(qb().selectFrom('users').orderBy('name').toSQL())).toContain('ORDER BY')
	89	+ expect(String(qb().selectFrom('users').orderBy('users.name').toSQL())).toContain('ORDER BY')
	90	+ expect(String(qb().selectFrom('users').select(['name AS n']).toSQL())).toContain('name')
	91	+ expect(String(qb().selectFrom('users').groupBy('team_id').toSQL())).toContain('GROUP BY')
	92	+ })
	93	+
	94	+ it('star, in the one place it belongs', () => {
	95	+ // Allowed for select, refused for ORDER BY, where it is not a column.
	96	+ expect(String(qb().selectFrom('users').select(['*']).toSQL())).toContain('*')
	97	+ expect(String(qb().selectFrom('users').select(['users.*']).toSQL())).toContain('users')
	98	+ expect(() => qb().selectFrom('users').orderBy('*')).toThrow(REFUSED)
	99	+ })
	100	+
	101	+ it('addSelect still takes the array form', () => {
	102	+ expect(String(qb().selectFrom('users').addSelect(['name', 'team_id']).toSQL())).toContain('name')
	103	+ })
	104	+
	105	+ it('expressions go through raw, which is the escape hatch', () => {
	106	+ // The strict rule is only tenable because this exists: `raw` and the *Raw
	107	+ // methods escape what they interpolate, and bypass the identifier check by
	108	+ // design rather than by accident.
	109	+ const sql = String(qb().selectFrom('users').select([raw('COUNT(*) AS c')]).toSQL())
	110	+ expect(sql).toContain('COUNT(*)')
	111	+ })
	112	+})