Skip to content

Commit 7846504

Browse files
committed
harden YAML guard: charge keys + escaping, bound stack, fail closed
Addresses review findings on the alias-expansion guard: - Charge object keys, not just values. JSON.stringify re-emits every key on each alias expansion, so an aliased object with a long key amplified without being counted. Keys are now charged against the size cap. - Compute the exact JSON-escaped string length (quotes, backslashes, control chars) instead of a flat multiplier. Plain text is charged its true 1:1 size (no false rejection of large legitimate documents) while escape-heavy strings are charged their real, larger cost (no cap bypass). - Count each node as it is enqueued and only push container nodes onto the traversal stack. A pathologically wide fan-out now trips a cap during the enqueue loop instead of first materializing millions of stack entries and exhausting memory inside the guard itself. - Fail closed in POST /api/files/parse: a YamlComplexityError rejection is now re-thrown (mirroring isPayloadSizeLimitError) rather than silently falling back to storing the crafted document as raw text.
1 parent 6ee6a9b commit 7846504

3 files changed

Lines changed: 119 additions & 17 deletions

File tree

apps/sim/app/api/files/parse/route.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { checkInternalAuth } from '@/lib/auth/hybrid'
1313
import { sanitizeUrlForLog } from '@/lib/core/utils/logging'
1414
import { assertKnownSizeWithinLimit, isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
1515
import { isSupportedFileType, parseFile } from '@/lib/file-parsers'
16+
import { isYamlComplexityError } from '@/lib/file-parsers/yaml-parser'
1617
import { isUsingCloudStorage, StorageService } from '@/lib/uploads'
1718
import { uploadExecutionFile } from '@/lib/uploads/contexts/execution'
1819
import {
@@ -1043,6 +1044,9 @@ async function handleGenericTextBuffer(
10431044
}
10441045
} catch (parserError) {
10451046
if (isPayloadSizeLimitError(parserError)) throw parserError
1047+
// Fail closed on a resource-exhaustion rejection instead of silently
1048+
// storing the crafted document as raw text.
1049+
if (isYamlComplexityError(parserError)) throw parserError
10461050

10471051
logger.warn('Specialized parser failed, falling back to generic parsing:', parserError)
10481052
}

apps/sim/lib/file-parsers/yaml-parser.test.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,22 @@ describe('parseYAMLBuffer', () => {
5050
await expect(parseYAMLBuffer(Buffer.from(bomb))).rejects.toBeInstanceOf(YamlComplexityError)
5151
})
5252

53+
it('rejects a key-amplification bomb (aliased object with a long key)', async () => {
54+
const longKey = 'k'.repeat(2000)
55+
const lines: string[] = [`l0: &l0 {${longKey}: 1}`]
56+
for (let i = 1; i <= 8; i++) {
57+
lines.push(
58+
`l${i}: &l${i} [${Array(10)
59+
.fill(`*l${i - 1}`)
60+
.join(',')}]`
61+
)
62+
}
63+
lines.push(`root: [${Array(10).fill('*l8').join(',')}]`)
64+
const bomb = lines.join('\n')
65+
expect(Buffer.byteLength(bomb)).toBeLessThan(4096)
66+
await expect(parseYAMLBuffer(Buffer.from(bomb))).rejects.toBeInstanceOf(YamlComplexityError)
67+
})
68+
5369
it('surfaces malformed YAML as an Invalid YAML error', async () => {
5470
await expect(parseYAMLBuffer(Buffer.from('key: "unterminated\n'))).rejects.toThrow(
5571
/Invalid YAML/
@@ -75,4 +91,25 @@ describe('assertYamlWithinLimits', () => {
7591
node.self = node
7692
expect(() => assertYamlWithinLimits(node)).toThrow(YamlComplexityError)
7793
})
94+
95+
it('charges object keys, not just values', () => {
96+
const hugeKey = 'k'.repeat(70 * 1024 * 1024)
97+
expect(() => assertYamlWithinLimits({ [hugeKey]: 1 })).toThrow(YamlComplexityError)
98+
})
99+
100+
it('rejects a large plain-text value only when it truly exceeds the size cap', () => {
101+
// ~10 MB of plain ASCII serializes ~1:1 and must be accepted (no false positive).
102+
expect(() => assertYamlWithinLimits({ text: 'a'.repeat(10 * 1024 * 1024) })).not.toThrow()
103+
// ~70 MB exceeds the 64 MB cap and must be rejected.
104+
expect(() => assertYamlWithinLimits({ text: 'a'.repeat(70 * 1024 * 1024) })).toThrow(
105+
YamlComplexityError
106+
)
107+
})
108+
109+
it('charges the true escaped length of escape-heavy strings', () => {
110+
// ~11M control chars each serialize to a six-char \uXXXX escape (~66 MB > 64 MB).
111+
const escapeHeavy = ''.repeat(11 * 1024 * 1024)
112+
expect(() => assertYamlWithinLimits({ text: escapeHeavy })).toThrow(YamlComplexityError)
113+
expect(escapeHeavy.length).toBeLessThan(64 * 1024 * 1024)
114+
})
78115
})

apps/sim/lib/file-parsers/yaml-parser.ts

Lines changed: 78 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -42,60 +42,121 @@ export class YamlComplexityError extends Error {
4242
}
4343

4444
/**
45-
* Estimate the pretty-printed (`JSON.stringify(value, null, 2)`) byte cost a
46-
* single node contributes, including the indentation/newline overhead that
47-
* dominates deeply nested alias bombs.
45+
* Type guard for {@link YamlComplexityError}. Callers use this to fail closed on
46+
* a complexity-limit rejection instead of falling back to a generic parse.
47+
*/
48+
export function isYamlComplexityError(error: unknown): error is YamlComplexityError {
49+
return error instanceof YamlComplexityError
50+
}
51+
52+
/**
53+
* Exact serialized length (in UTF-16 code units — the unit V8 allocates for the
54+
* resulting string) that `JSON.stringify` produces for a string, accounting for
55+
* the escape expansion of quotes, backslashes, and control characters. Computed
56+
* precisely rather than with a flat multiplier so plain text is charged its
57+
* true size (no false rejection of large legitimate documents) while
58+
* escape-heavy strings are charged their real, larger cost (no cap bypass).
59+
*/
60+
function serializedStringLength(value: string): number {
61+
let length = 2 // surrounding quotes
62+
for (let i = 0; i < value.length; i++) {
63+
const code = value.charCodeAt(i)
64+
if (code === 0x22 /* " */ || code === 0x5c /* \ */) {
65+
length += 2
66+
} else if (code < 0x20) {
67+
// \b \t \n \f \r use two-char escapes; other control chars use \uXXXX (six)
68+
length +=
69+
code === 0x08 || code === 0x09 || code === 0x0a || code === 0x0c || code === 0x0d ? 2 : 6
70+
} else {
71+
length += 1
72+
}
73+
}
74+
return length
75+
}
76+
77+
/**
78+
* Estimate the pretty-printed (`JSON.stringify(value, null, 2)`) size a single
79+
* value node contributes, including the indentation/newline overhead that
80+
* dominates deeply nested alias bombs and the exact escape expansion of strings.
4881
*/
4982
function estimateNodeBytes(value: unknown, depth: number): number {
5083
const indentOverhead = depth * 2 + 4
51-
if (typeof value === 'string') return indentOverhead + value.length + 2
84+
if (typeof value === 'string') return indentOverhead + serializedStringLength(value)
5285
return indentOverhead + 16
5386
}
5487

88+
/**
89+
* Estimate the serialized size of an object key (`"key": `). Keys are re-emitted
90+
* on every alias expansion of their parent object, so an aliased object with a
91+
* long key amplifies just like an aliased value — this must be charged or the
92+
* size cap is trivially bypassed.
93+
*/
94+
function estimateKeyBytes(key: string): number {
95+
return serializedStringLength(key) + 2 // ": "
96+
}
97+
5598
/**
5699
* Iteratively walk the parsed YAML value with strict node-count, output-size,
57100
* and depth limits, returning the document depth. Repeated (aliased) references
58101
* are intentionally counted each time they are reached, mirroring the way
59102
* `JSON.stringify` expands them — this is what makes the alias-expansion bomb
60103
* detectable before serialization.
61104
*
105+
* Each node is charged against the caps as it is *enqueued*, before its own
106+
* children are pushed, and only container nodes are pushed onto the traversal
107+
* stack. A pathologically wide fan-out (e.g. an array of millions of aliases)
108+
* therefore trips a cap during the enqueue loop instead of first materializing
109+
* millions of stack entries and exhausting memory inside the guard itself.
110+
*
62111
* @throws {YamlComplexityError} when any limit is exceeded
63112
*/
64113
export function assertYamlWithinLimits(root: unknown): number {
65114
let visited = 0
66115
let estimatedBytes = 0
67116
let maxDepth = 0
68117

69-
const stack: Array<{ value: unknown; depth: number }> = [{ value: root, depth: 0 }]
70-
71-
while (stack.length > 0) {
72-
const { value, depth } = stack.pop()!
73-
118+
const charge = (bytes: number): void => {
74119
if (++visited > MAX_YAML_EXPANDED_NODES) {
75120
throw new YamlComplexityError(
76121
`YAML document exceeds the maximum of ${MAX_YAML_EXPANDED_NODES} expanded nodes (possible alias-expansion bomb)`
77122
)
78123
}
79-
80-
estimatedBytes += estimateNodeBytes(value, depth)
124+
estimatedBytes += bytes
81125
if (estimatedBytes > MAX_YAML_SERIALIZED_BYTES) {
82126
throw new YamlComplexityError(
83127
`YAML document expands beyond the maximum serialized size of ${MAX_YAML_SERIALIZED_BYTES} bytes (possible alias-expansion bomb)`
84128
)
85129
}
130+
}
131+
132+
const isContainer = (value: unknown): value is object =>
133+
value !== null && typeof value === 'object'
86134

87-
if (value === null || typeof value !== 'object') continue
135+
charge(estimateNodeBytes(root, 0))
136+
const stack: Array<{ value: object; depth: number }> = []
137+
if (isContainer(root)) stack.push({ value: root, depth: 0 })
138+
139+
while (stack.length > 0) {
140+
const { value, depth } = stack.pop()!
141+
const childDepth = depth + 1
88142

89-
if (depth + 1 > maxDepth) maxDepth = depth + 1
90-
if (depth + 1 > MAX_YAML_DEPTH) {
143+
if (childDepth > maxDepth) maxDepth = childDepth
144+
if (childDepth > MAX_YAML_DEPTH) {
91145
throw new YamlComplexityError(
92146
`YAML document exceeds the maximum nesting depth of ${MAX_YAML_DEPTH}`
93147
)
94148
}
95149

96-
const children = Array.isArray(value) ? value : Object.values(value as Record<string, unknown>)
97-
for (const child of children) {
98-
stack.push({ value: child, depth: depth + 1 })
150+
if (Array.isArray(value)) {
151+
for (const child of value) {
152+
charge(estimateNodeBytes(child, childDepth))
153+
if (isContainer(child)) stack.push({ value: child, depth: childDepth })
154+
}
155+
} else {
156+
for (const [key, child] of Object.entries(value as Record<string, unknown>)) {
157+
charge(estimateKeyBytes(key) + estimateNodeBytes(child, childDepth))
158+
if (isContainer(child)) stack.push({ value: child, depth: childDepth })
159+
}
99160
}
100161
}
101162

0 commit comments

Comments
 (0)