Skip to content

Commit 21cac0a

Browse files
[CodeQL] String length validation for API Routes (elastic#270485)
* Introduces a new CodeQL rule to alert on API routes that use either `@kbn/config-schema` or `zod` in a way that allows for strings of unbounded length. This is a companion to our existing CodeQL rule that alerts on unbounded arrays. * Introduces a [file exclusion pattern](https://github.com/legrego/kibana/blob/87f43b9bc3bc1a78b718cbb845861de32e53c3e7/.github/codeql/custom-queries/dos/KibanaDoSExclusions.qll) for both of our DoS CodeQL rules to allow us to more systematically omit files from alerting on these findings, if it is clear that their usage of these validation schemas is not for the purpose of API route validation. --------- Co-authored-by: Elena Shostak <165678770+elena-shostak@users.noreply.github.com>
1 parent 569cfcc commit 21cac0a

24 files changed

Lines changed: 1559 additions & 47 deletions

File tree

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
/**
2+
* Shared predicates for excluding non-request-payload schema usages from
3+
* Kibana DoS queries (unbounded strings, unbounded arrays, etc.).
4+
*
5+
* CONSERVATIVE POLICY: Only exclude file paths that NEVER contain HTTP request
6+
* payload schemas. When in doubt, keep the finding — a false positive is far
7+
* less costly than missing a real vulnerability.
8+
*
9+
* For one-off false positives in mixed-purpose files, use inline suppression:
10+
* // codeql[js/kibana/unbounded-string-in-schema] reason
11+
* // codeql[js/kibana/unbounded-array-in-schema] reason
12+
*
13+
* If a new file category consistently produces false positives, add a path
14+
* pattern here rather than scattering per-line suppressions.
15+
*/
16+
17+
import javascript
18+
19+
/**
20+
* Holds when `e` resides in a file whose schemas are known to never validate
21+
* HTTP request payloads. These fall into structural/data-at-rest categories:
22+
* plugin configuration, saved-object attributes, UI settings, content
23+
* management layer schemas, sample-data registration, AI/LLM output schemas,
24+
* and tooling config.
25+
*/
26+
predicate shouldExcludeFileFromDoSRules(Expr e) {
27+
exists(string path | path = e.getFile().getRelativePath() |
28+
// Plugin configuration (kibana.yml settings)
29+
e.getFile().getBaseName() = "config.ts"
30+
or
31+
// Plugin server entry points (re-exports only)
32+
(e.getFile().getBaseName() = "index.ts" and
33+
path.regexpMatch(".*/plugins/[^/]+(/[^/]+)?/server/index\\.ts"))
34+
or
35+
// Saved-object attribute schemas (versioned shapes, never route payloads)
36+
path.regexpMatch(".*/saved_objects/schemas/.*")
37+
or
38+
// Saved-object model-version migration schemas
39+
path.regexpMatch(".*/saved_objects/model_versions/.*")
40+
or
41+
// Saved-object type sub-schemas (e.g. cases/server/saved_object_types/*/schemas/*)
42+
path.regexpMatch(".*/saved_object_types/.*/schemas/.*")
43+
or
44+
// Dashboard saved-object attribute schemas
45+
path.regexpMatch(".*/dashboard_saved_object/schema/.*")
46+
or
47+
// Content-management layer schemas (maps, lens, links CM CRUD)
48+
path.regexpMatch(".*/content_management/schema/.*")
49+
or
50+
// UI-settings definitions (Advanced Settings value schemas)
51+
e.getFile().getBaseName() = "ui_settings.ts"
52+
or
53+
path.regexpMatch(".*/ui_settings/.*")
54+
or
55+
// Sample-data registration schema (internal registration, not HTTP input)
56+
path.regexpMatch(".*/sample_data/lib/sample_dataset_schema\\.ts")
57+
or
58+
// Connector-schema type-only files (no route handlers)
59+
path.regexpMatch(".*/kbn-connector-schemas/.*/types/.*")
60+
or
61+
// LLM/AI structured-output schemas
62+
path.regexpMatch(".*/compaction_schema\\.ts")
63+
or
64+
// Agent-builder tool parameter schemas (AI tool arguments, not HTTP routes)
65+
path.regexpMatch(".*/agent_builder/tools/.*")
66+
or
67+
// Benchmark tooling config schemas
68+
path.regexpMatch(".*/kbn-bench/.*")
69+
or
70+
// Scout test-environment config schemas
71+
path.regexpMatch(".*/kbn-scout/.*/schema/.*")
72+
)
73+
}

.github/codeql/custom-queries/dos/UnboundedArrayInRoute.md

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ An attacker could exploit unbounded array validation by sending extremely large
1111
- Application crashes due to out-of-memory conditions
1212
- Degraded performance affecting all users of the system
1313

14-
The query specifically targets `schema.arrayOf()` calls that are missing the `maxSize` option in the second argument. It excludes configuration files (`config.ts`) and plugin entry points (`server/index.ts`) as these typically handle trusted internal configuration rather than external user input.
14+
The query specifically targets `schema.arrayOf()` calls that are missing the `maxSize` option in the second argument. It uses a shared exclusion list (defined in `KibanaDoSExclusions.qll`) to skip files whose schemas are known to never validate HTTP request payloads, such as saved-object attribute schemas, plugin configuration, UI settings definitions, content-management layer schemas, and similar structural/data-at-rest categories. See that file for the full set of excluded path patterns.
1515

1616
## Recommendation
1717

@@ -119,6 +119,19 @@ router.post({
119119
}, handler);
120120
```
121121

122+
## False positives and suppression
123+
124+
This query intentionally casts a wide net — it flags all unbounded `schema.arrayOf()` calls except those in file paths that are clearly non-payload contexts. Some findings will be in schemas that validate route **responses**, saved-object attributes in shared files, or other non-request-payload contexts. These are expected false positives.
125+
126+
To suppress a legitimate false positive, add a `codeql[...]` comment on the line above the flagged call:
127+
128+
```javascript
129+
// codeql[js/kibana/unbounded-array-in-schema] internal registration — not route input
130+
schema.arrayOf(schema.string())
131+
```
132+
133+
The exclusion list in `KibanaDoSExclusions.qll` is maintained conservatively and may be updated as new non-payload schema patterns are identified. If you encounter a false positive that affects an entire file category (not a one-off), consider proposing an addition to the shared exclusion library rather than adding per-line suppressions.
134+
122135
## References
123136

124137
- [OWASP: Denial of Service Attacks](https://owasp.org/www-community/attacks/Denial_of_Service)

.github/codeql/custom-queries/dos/UnboundedArrayInRoute.qhelp

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,19 @@ requests. This can cause:
1919
<li>Degraded performance affecting all users of the system</li>
2020
</ul>
2121
<p>
22-
The query specifically targets <code>schema.arrayOf()</code> calls that are missing the
23-
<code>maxSize</code> option in the second argument. It excludes configuration files
24-
(<code>config.ts</code>) and plugin entry points (<code>server/index.ts</code>) as these
25-
typically handle trusted internal configuration rather than external user input.
22+
The query specifically targets <code>schema.arrayOf()</code> calls that are missing the
23+
<code>maxSize</code> option in the second argument. It uses a shared exclusion list (defined
24+
in <code>KibanaDoSExclusions.qll</code>) to skip files whose schemas are known to never validate
25+
HTTP request payloads, such as saved-object attribute schemas, plugin configuration, UI settings
26+
definitions, content-management layer schemas, and similar structural/data-at-rest categories.
27+
See that file for the full set of excluded path patterns.
28+
</p>
29+
<p>
30+
Some findings will be in schemas that validate route <b>responses</b> or other non-request-payload
31+
contexts. These are expected false positives. To suppress a legitimate false positive, add a
32+
<code>codeql[js/kibana/unbounded-array-in-schema]</code> comment on the line above the flagged
33+
call. If a false positive affects an entire file category, consider proposing an addition to
34+
<code>KibanaDoSExclusions.qll</code> rather than adding per-line suppressions.
2635
</p>
2736
</overview>
2837

.github/codeql/custom-queries/dos/UnboundedArrayInRoute.ql

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
*/
1515

1616
import javascript
17+
import dos.KibanaDoSExclusions
1718

1819
/**
1920
* Gets the local variable bound to 'schema' imported from '@kbn/config-schema'
@@ -22,7 +23,11 @@ LocalVariable schemaVariable() {
2223
exists(ImportDeclaration decl, ImportSpecifier spec |
2324
decl.getImportedPathExpr().getStringValue() = "@kbn/config-schema" and
2425
spec = decl.getASpecifier() and
25-
spec.getImportedName() = "schema" and
26+
(
27+
spec.getImportedName() = "schema"
28+
or
29+
spec instanceof ImportNamespaceSpecifier
30+
) and
2631
result = spec.getLocal().getVariable()
2732
)
2833
}
@@ -54,25 +59,10 @@ class SchemaArrayOfCall extends CallExpr {
5459
}
5560
}
5661

57-
/**
58-
* Identifies files that should be excluded from analysis:
59-
* - config.ts files (plugin configuration, not request validation)
60-
* - index.ts files at plugin server root (typically re-exports)
61-
*/
62-
predicate isInExcludedFile(Expr e) {
63-
e.getFile().getBaseName() = "config.ts"
64-
or
65-
// Exclude index.ts at plugin server root: plugins/*/server/index.ts or plugins/*/*/server/index.ts
66-
(
67-
e.getFile().getBaseName() = "index.ts" and
68-
e.getFile().getRelativePath().regexpMatch(".*/plugins/[^/]+(/[^/]+)?/server/index\\.ts")
69-
)
70-
}
71-
7262
from SchemaArrayOfCall arrayCall
7363
where
7464
not arrayCall.hasMaxSize() and
75-
not isInExcludedFile(arrayCall)
65+
not shouldExcludeFileFromDoSRules(arrayCall)
7666
select arrayCall,
7767
"This schema.arrayOf() call does not specify a maxSize. Unbounded input can cause Denial of Service (DoS) vulnerabilities. Consider adding { maxSize: N } as the second argument."
7868

0 commit comments

Comments
 (0)