Skip to content

Commit c8ddfa5

Browse files
committed
fix(mssql): reject a parenthesised or negated constant tautology in a WHERE clause
The shared guard recognises `OR 1` but not `OR (1)`, `OR ((1))`, `OR NOT 0`, or `OR NOT (FALSE)` — a parenthesis or a NOT between the operator and the constant hides it. Both patterns require the constant to be the whole parenthesised term, so a real disjunct such as `OR (1 = priority)` is untouched. This narrows the gap rather than closing it, and is not meant to close it: an always-true expression is not lexically decidable in general, which is why the WHERE screen stays documented as defense-in-depth rather than a boundary.
1 parent c6fd72f commit c8ddfa5

2 files changed

Lines changed: 44 additions & 0 deletions

File tree

apps/sim/app/api/tools/mssql/utils.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,29 @@ describe('buildUpdateQuery / buildDeleteQuery WHERE screening', () => {
145145
expect(() => buildDeleteQuery('dbo.users', `[a"] = 1 OR 1=1`)).toThrow(/bracketed identifier/)
146146
})
147147

148+
/**
149+
* The shared guard sees `OR 1` but not a parenthesised or negated constant.
150+
* These are the specific forms it documents as undetected; the class as a
151+
* whole is not lexically decidable, so this narrows rather than closes it.
152+
*/
153+
it.each([
154+
'id = 1 OR (1)',
155+
'id = 1 OR ((1))',
156+
'id = 1 OR NOT 0',
157+
'id = 1 OR NOT (0)',
158+
'id = 1 OR (TRUE)',
159+
])('rejects the constant tautology %s', (where) => {
160+
expect(() => buildDeleteQuery('dbo.users', where)).toThrow()
161+
})
162+
163+
it.each([
164+
'id = 1 OR (priority = 2)',
165+
'id = 1 OR (1 = priority)',
166+
"status = 'open' OR (retries < 3)",
167+
])('still accepts the real disjunct %s', (where) => {
168+
expect(() => buildDeleteQuery('dbo.users', where)).not.toThrow()
169+
})
170+
148171
it.each([
149172
['a semicolon-less batch', "id = 1 DBCC SHRINKDATABASE('app')"],
150173
['an appended SELECT', 'id = 1 SELECT secret FROM dbo.credentials'],

apps/sim/app/api/tools/mssql/utils.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,26 @@ const MSSQL_WHERE_SELECT = /\bselect\b/i
420420
* views still reachable as `master..sysobjects`.
421421
* @see https://learn.microsoft.com/en-us/sql/relational-databases/system-catalog-views/catalog-views-transact-sql
422422
*/
423+
/**
424+
* Constant tautologies the shared guard's `OR <literal>` rule cannot see because
425+
* a parenthesis or a `NOT` sits between the operator and the constant —
426+
* `OR (1)`, `OR ((1))`, `OR NOT 0`, `OR NOT (FALSE)`.
427+
*
428+
* Both patterns require the constant to be the *whole* parenthesised term, so a
429+
* real disjunct is untouched: `OR (1 = priority)` does not match, because a
430+
* closing paren does not follow the digit.
431+
*
432+
* This narrows the gap; it does not close it, and it is not meant to. An
433+
* always-true expression cannot be recognised lexically in general —
434+
* `OR 2 > 1`, `OR LEN(x) >= 0`, and `OR id IS NOT NULL` all survive any pattern
435+
* list — which is why {@link validateWhereClause} is documented as
436+
* defense-in-depth rather than a boundary.
437+
*/
438+
const MSSQL_WHERE_CONSTANT_TAUTOLOGY: readonly RegExp[] = [
439+
/\bor\s+(?:not\s+)*\(+\s*(?:\d+(?:\.\d+)?|true|false)\s*\)+/i,
440+
/\bor\s+not\s+(?:\d+(?:\.\d+)?|true|false)\b/i,
441+
]
442+
423443
const MSSQL_CATALOG_PATTERNS: readonly RegExp[] = [
424444
/information_schema/i,
425445
/\bsys\./i,
@@ -469,6 +489,7 @@ function validateWhereClause(where: string): void {
469489
MSSQL_STATEMENT_KEYWORDS.test(masked) ||
470490
MSSQL_PROCEDURE_PATTERN.test(masked) ||
471491
MSSQL_WHERE_SELECT.test(masked) ||
492+
MSSQL_WHERE_CONSTANT_TAUTOLOGY.some((pattern) => pattern.test(masked)) ||
472493
MSSQL_CATALOG_PATTERNS.some((pattern) => pattern.test(masked))
473494
) {
474495
throw new Error('WHERE clause contains potentially dangerous operation')

0 commit comments

Comments
 (0)