Skip to content

Commit 085e3ae

Browse files
committed
fix(egress): fold NAT64 addresses, and parse the boundary check properly
A DNS64 resolver returns an IPv4 destination wrapped in the RFC 6052 well-known prefix, so `64:ff9b::a9fe:a9fe` is the metadata endpoint. Canonicalization did not recognise that, and an allowlisted hostname resolving there was permitted — probed and confirmed. Narrower than it first looked: `isPrivateIp` already rejects every NAT64 form, so an ordinary destination was never exposed; the hole was only for a vouched one, where the class check is skipped and the metadata exception is all that stands in the way. Folding it also makes an operator's IPv4 range match the NAT64 spelling of an address inside it. The boundary check is parsed with the TypeScript AST instead of matched with a regex. Two rounds found holes in both directions — a comment or string naming a transport reported a violation that did not exist, and a regex literal containing a quote hid one that did — which is what a scanner that does not understand the grammar will keep doing. A template interpolation was the third. Parsing surfaced a false positive the regex never had: `import { type X } from 'undici'` is elided under `verbatimModuleSyntax: false`, so it cannot load anything. Elision is now modelled properly — a default or namespace binding keeps an import alive, an all-type named import does not. Eleven forms verified by probe. That needs `@typescript/typescript6` declared at the root rather than relied on by hoisting from apps/sim. It adds no new `tsc` bin, and check:native-typecheck still reports 7.0.2. Docs: the provenance table was missing `proxy`, and the line about naming a destination read as though the allowlist reaches everything. It does not reach a content fetch or a proxy — a proxy must be public, since it decides where every other request may go.
1 parent bb00376 commit 085e3ae

6 files changed

Lines changed: 123 additions & 112 deletions

File tree

apps/docs/content/docs/platform/self-hosting/security.mdx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,8 +143,9 @@ Sim blocks outbound requests to private, reserved, and loopback addresses. This
143143
| Request target | The HTTP block's URL, an A2A agent, an RSS feed, a Function block's `fetch` | Yes |
144144
| Database host | A database, cache, or mail connector's host | Yes |
145145
| Content fetch | An image URL, a file imported by URL, a link from a third-party API response | **No** |
146+
| Proxy | The outbound HTTP proxy itself | **No** |
146147

147-
Content fetches never reach a private destination, allowlist or not — that is the class where SSRF is actually exploited.
148+
Content fetches never reach a private destination, allowlist or not — that is the class where SSRF is actually exploited. Nor does the proxy: it is the component deciding where everything else may go, so it is held to public destinations regardless of what the allowlist says.
148149

149150
Deployments frequently need to reach an internal service by name or address. Name the destinations:
150151

@@ -157,6 +158,8 @@ A wildcard (`*.svc.cluster.local`) and a broad range (`10.0.0.0/8`) are accepted
157158

158159
Naming a destination permits plain HTTP to it and lifts the blocked-port list for it, since those are the same decision about the same host. Cloud metadata endpoints (`169.254.169.254` and equivalents) stay blocked no matter how broad the allowlist is, and both variables are ignored entirely on Sim Cloud.
159160

161+
The allowlist reaches the four provenances marked **Yes** above. It does not reach a content fetch, and it does not reach a proxy: an HTTP block's `proxyUrl` must be a public address, because the proxy is what decides where every other request may go. Adding an internal proxy to the allowlist will not make it work.
162+
160163
To reach a service on the Docker host, pair the allowlist with the host alias that Compose already sets up:
161164

162165
```bash

bun.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@
136136
"@clack/prompts": "1.7.0",
137137
"@octokit/rest": "^21.0.0",
138138
"@types/opentype.js": "1.3.10",
139+
"@typescript/typescript6": "^6.0.2",
139140
"@typescript/native": "npm:typescript@^7.0.2",
140141
"@vercel/og": "0.6.8",
141142
"chalk": "5.6.2",

packages/security/src/egress.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,14 @@ describe('cloud metadata is never reachable', () => {
100100
)
101101
})
102102

103+
it.each([
104+
['64:ff9b::a9fe:a9fe', 'the NAT64 form a DNS64 resolver returns'],
105+
['64:ff9b::169.254.169.254', 'NAT64 written long-hand'],
106+
])('blocks %s through an allowlisted hostname — %s', (address) => {
107+
const permissive = createEgressPolicy({ allowedHosts: 'internal.corp' })
108+
expect(reason(permissive, 'https://internal.corp/', address)).toBe('address-metadata')
109+
})
110+
103111
it('blocks the AWS IPv6 metadata address', () => {
104112
expect(reason(hosted, 'https://[fd00:ec2::254]/')).toBe('address-metadata')
105113
})
@@ -163,6 +171,7 @@ describe('an IPv4 range matches every spelling of the same address', () => {
163171
['::a00:1', 'the IPv4-compatible form a resolver can return'],
164172
['::ffff:10.0.0.1', 'the IPv4-mapped form'],
165173
['::10.0.0.1', 'IPv4-compatible written long-hand'],
174+
['64:ff9b::a00:1', 'the NAT64 form'],
166175
])('permits %s — %s', (address) => {
167176
expect(decide(ranged, 'https://svc.internal/', address).allowed).toBe(true)
168177
})

packages/security/src/egress.ts

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -263,26 +263,41 @@ function matchesRangeAllowlist(address: string, policy: EgressPolicy): boolean {
263263
* URL parser normalizes to `::a9fe:a9fe`), so comparing without this misses the
264264
* metadata endpoint written that way.
265265
*/
266+
function embeddedIpv4(parts: readonly number[]): string {
267+
return ipaddr
268+
.fromByteArray([
269+
(parts[6] >> 8) & 0xff,
270+
parts[6] & 0xff,
271+
(parts[7] >> 8) & 0xff,
272+
parts[7] & 0xff,
273+
])
274+
.toString()
275+
}
276+
277+
/** RFC 6052 well-known NAT64 prefix, `64:ff9b::/96`. */
278+
const NAT64_WELL_KNOWN_PREFIX = [0x0064, 0xff9b, 0, 0, 0, 0] as const
279+
266280
function canonicalAddress(address: string): string | null {
267281
const clean = unwrapIpv6Brackets(address)
268282
if (!ipaddr.isValid(clean)) return null
269283

270284
const parsed = ipaddr.process(clean)
271285
if (parsed.kind() === 'ipv6') {
272286
const parts = (parsed as ipaddr.IPv6).parts
287+
288+
// A DNS64 resolver hands back the IPv4 destination wrapped in the well-known
289+
// NAT64 prefix. Left unfolded, `64:ff9b::a9fe:a9fe` does not read as the
290+
// metadata endpoint it is, and a vouched destination would reach it.
291+
if (NAT64_WELL_KNOWN_PREFIX.every((part, index) => parts[index] === part)) {
292+
return embeddedIpv4(parts)
293+
}
294+
273295
const embedded = ((parts[6] << 16) >>> 0) + parts[7]
274296
// `::` and `::1` are the unspecified and loopback addresses, not an IPv4
275297
// carried inside IPv6 — folding them would turn `::1` into `0.0.0.1` and
276298
// stop an operator's `::1/128` entry matching it.
277299
if (parts.slice(0, 6).every((part) => part === 0) && embedded > 1) {
278-
return ipaddr
279-
.fromByteArray([
280-
(parts[6] >> 8) & 0xff,
281-
parts[6] & 0xff,
282-
(parts[7] >> 8) & 0xff,
283-
parts[7] & 0xff,
284-
])
285-
.toString()
300+
return embeddedIpv4(parts)
286301
}
287302
}
288303
return parsed.toString()

scripts/check-egress-boundary.ts

Lines changed: 85 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,24 @@
99
* `undici` directly gets none of that, and the omission is invisible — the code
1010
* works, it just has no guard.
1111
*
12-
* This checks the import edge rather than the call, because that is the part
12+
* This checks the module edge rather than the call, because that is the part
1313
* that cannot be hidden behind a helper.
1414
*
15+
* Parsed with the TypeScript AST rather than matched with a regex. Two rounds of
16+
* review found regex holes in both directions — a comment or string naming a
17+
* transport reported a violation that was not there, and a regex literal
18+
* containing a quote hid a real one — which is what a scanner that does not
19+
* understand the grammar will keep doing.
20+
*
1521
* Not checked: bare `fetch()`. It is used constantly for same-origin and
1622
* server-action calls where the guard does not apply, so flagging it would be
17-
* noise. The transports it can reach are covered by the rules above.
23+
* noise. The transports it can reach are covered by the rules below.
1824
*
1925
* Usage: bun run scripts/check-egress-boundary.ts
2026
*/
2127
import { readdirSync, readFileSync } from 'node:fs'
2228
import path from 'node:path'
29+
import ts from '@typescript/typescript6'
2330

2431
const ROOT = path.resolve(import.meta.dir, '..')
2532

@@ -36,31 +43,15 @@ const SCAN_DIRS = [
3643
const SKIP_DIRS = new Set(['node_modules', 'dist', '.next', '.turbo', 'coverage'])
3744

3845
/** Modules that can open a socket directly. */
39-
const TRANSPORTS = ['http', 'https', 'undici', 'http-proxy-agent', 'https-proxy-agent']
40-
41-
const MODULE_ALTERNATION = TRANSPORTS.map((name) => name.replace(/[-]/g, '\\-')).join('|')
42-
const SPECIFIER = `['"](?:node:)?(?:${MODULE_ALTERNATION})['"]`
43-
44-
/**
45-
* Every way a module reaches one of these at runtime.
46-
*
47-
* Matched against the whole source rather than line by line, because an import
48-
* list broken across lines would otherwise slip past. `import type` is excluded:
49-
* a type has no runtime presence and cannot open anything.
50-
*/
51-
const RUNTIME_LOADS: ReadonlyArray<{ pattern: RegExp; kind: string }> = [
52-
{
53-
pattern: new RegExp(`^[ \t]*import\\s+(?!type\\s)[\\s\\S]*?from\\s*${SPECIFIER}`, 'gm'),
54-
kind: 'import',
55-
},
56-
{ pattern: new RegExp(`^[ \t]*import\\s*${SPECIFIER}`, 'gm'), kind: 'side-effect import' },
57-
{
58-
pattern: new RegExp(`^[ \t]*export\\s+(?!type\\s)[\\s\\S]*?from\\s*${SPECIFIER}`, 'gm'),
59-
kind: 're-export',
60-
},
61-
{ pattern: new RegExp(`\\bimport\\s*\\(\\s*${SPECIFIER}\\s*\\)`, 'g'), kind: 'dynamic import' },
62-
{ pattern: new RegExp(`\\brequire\\s*\\(\\s*${SPECIFIER}\\s*\\)`, 'g'), kind: 'require' },
63-
]
46+
const TRANSPORTS = new Set([
47+
'http',
48+
'https',
49+
'node:http',
50+
'node:https',
51+
'undici',
52+
'http-proxy-agent',
53+
'https-proxy-agent',
54+
])
6455

6556
/**
6657
* Modules allowed to hold a transport import, each because it *is* part of the
@@ -75,58 +66,6 @@ const ALLOWED = new Set([
7566
'apps/sim/lib/core/utils/fetch-deadline.ts',
7667
])
7768

78-
/**
79-
* Blanks comment bodies, preserving byte offsets so reported line numbers stay
80-
* exact. Without this the rules match their own documentation: a comment warning
81-
* against `require('undici')` reads identically to the call.
82-
*
83-
* Strings are deliberately left intact — the module specifier is itself a string,
84-
* so blanking them would stop every rule matching anything. A match that starts
85-
* inside a string is rejected separately by {@link stringRanges}.
86-
*/
87-
function blankComments(source: string): string {
88-
const out = source.split('')
89-
let i = 0
90-
while (i < source.length) {
91-
const two = source.slice(i, i + 2)
92-
if (two === '//' || two === '/*') {
93-
const end =
94-
two === '//'
95-
? (source.indexOf('\n', i) + 1 || source.length + 1) - 1
96-
: source.indexOf('*/', i + 2) + 2 || source.length
97-
for (let j = i; j < end; j++) if (out[j] !== '\n') out[j] = ' '
98-
i = end
99-
continue
100-
}
101-
if (two[0] === '"' || two[0] === "'" || two[0] === '`') {
102-
let j = i + 1
103-
while (j < source.length && source[j] !== two[0]) j += source[j] === '\\' ? 2 : 1
104-
i = j + 1
105-
continue
106-
}
107-
i++
108-
}
109-
return out.join('')
110-
}
111-
112-
/** Half-open [start, end) ranges covering every string literal body. */
113-
function stringRanges(source: string): Array<[number, number]> {
114-
const ranges: Array<[number, number]> = []
115-
let i = 0
116-
while (i < source.length) {
117-
const ch = source[i]
118-
if (ch === '"' || ch === "'" || ch === '`') {
119-
let j = i + 1
120-
while (j < source.length && source[j] !== ch) j += source[j] === '\\' ? 2 : 1
121-
ranges.push([i + 1, j])
122-
i = j + 1
123-
continue
124-
}
125-
i++
126-
}
127-
return ranges
128-
}
129-
13069
function walk(dir: string, out: string[] = []): string[] {
13170
for (const entry of readdirSync(dir, { withFileTypes: true })) {
13271
if (SKIP_DIRS.has(entry.name)) continue
@@ -141,39 +80,82 @@ interface Violation {
14180
file: string
14281
line: number
14382
kind: string
144-
snippet: string
83+
specifier: string
84+
}
85+
86+
/**
87+
* Whether TypeScript drops this import at emit, leaving nothing that could load
88+
* the module.
89+
*
90+
* True for `import type … from 'm'` and for a named import whose every binding
91+
* is marked `type` — the repo compiles with `verbatimModuleSyntax: false`, so
92+
* that form is elided rather than kept as a side-effect import. A default or
93+
* namespace binding is a value and keeps the import alive, and a bare
94+
* `import 'm'` has no clause at all and always runs.
95+
*/
96+
function isElidedImport(node: ts.ImportDeclaration): boolean {
97+
const clause = node.importClause
98+
if (!clause) return false
99+
if (clause.isTypeOnly) return true
100+
if (clause.name) return false
101+
102+
const bindings = clause.namedBindings
103+
if (!bindings || !ts.isNamedImports(bindings)) return false
104+
return bindings.elements.every((element) => element.isTypeOnly)
105+
}
106+
107+
/**
108+
* Every runtime reference to a transport module. An import TypeScript elides is
109+
* skipped: it has no runtime presence and cannot open anything.
110+
*/
111+
function findTransportLoads(file: string, source: string): Array<Omit<Violation, 'file'>> {
112+
const sourceFile = ts.createSourceFile(file, source, ts.ScriptTarget.Latest, true)
113+
const found: Array<Omit<Violation, 'file'>> = []
114+
115+
const record = (node: ts.Node, specifier: string, kind: string) => {
116+
if (!TRANSPORTS.has(specifier)) return
117+
const { line } = sourceFile.getLineAndCharacterOfPosition(node.getStart(sourceFile))
118+
found.push({ line: line + 1, kind, specifier })
119+
}
120+
121+
const visit = (node: ts.Node): void => {
122+
if (ts.isImportDeclaration(node) && ts.isStringLiteralLike(node.moduleSpecifier)) {
123+
if (!isElidedImport(node)) {
124+
record(node, node.moduleSpecifier.text, node.importClause ? 'import' : 'side-effect import')
125+
}
126+
} else if (
127+
ts.isExportDeclaration(node) &&
128+
node.moduleSpecifier &&
129+
ts.isStringLiteralLike(node.moduleSpecifier) &&
130+
!node.isTypeOnly
131+
) {
132+
record(node, node.moduleSpecifier.text, 're-export')
133+
} else if (ts.isCallExpression(node)) {
134+
const isDynamicImport = node.expression.kind === ts.SyntaxKind.ImportKeyword
135+
const isRequire = ts.isIdentifier(node.expression) && node.expression.text === 'require'
136+
const argument = node.arguments[0]
137+
if ((isDynamicImport || isRequire) && argument && ts.isStringLiteralLike(argument)) {
138+
record(node, argument.text, isDynamicImport ? 'dynamic import' : 'require')
139+
}
140+
}
141+
ts.forEachChild(node, visit)
142+
}
143+
144+
visit(sourceFile)
145+
return found
145146
}
146147

147148
function main() {
148149
const violations: Violation[] = []
149150
let scanned = 0
150151

151152
for (const scanDir of SCAN_DIRS) {
152-
const abs = path.join(ROOT, scanDir)
153-
for (const file of walk(abs)) {
153+
for (const file of walk(path.join(ROOT, scanDir))) {
154154
const rel = path.relative(ROOT, file).split(path.sep).join('/')
155155
if (ALLOWED.has(rel)) continue
156156
scanned++
157-
const raw = readFileSync(file, 'utf8')
158-
const source = blankComments(raw)
159-
const strings = stringRanges(source)
160-
const insideString = (index: number) =>
161-
strings.some(([from, to]) => index >= from && index < to)
162-
163-
for (const { pattern, kind } of RUNTIME_LOADS) {
164-
pattern.lastIndex = 0
165-
for (const match of source.matchAll(pattern)) {
166-
if (match.index === undefined || insideString(match.index)) continue
167-
violations.push({
168-
file: rel,
169-
line: source.slice(0, match.index).split('\n').length,
170-
kind,
171-
snippet: raw
172-
.slice(match.index, match.index + match[0].length)
173-
.replace(/\s+/g, ' ')
174-
.trim(),
175-
})
176-
}
157+
for (const load of findTransportLoads(rel, readFileSync(file, 'utf8'))) {
158+
violations.push({ file: rel, ...load })
177159
}
178160
}
179161
}
@@ -186,7 +168,7 @@ function main() {
186168
console.error('✗ check-egress-boundary: raw HTTP transport outside the egress guard\n')
187169
for (const violation of violations) {
188170
console.error(` ${violation.file}:${violation.line} (${violation.kind})`)
189-
console.error(` ${violation.snippet}`)
171+
console.error(` ${violation.specifier}`)
190172
}
191173
console.error(
192174
'\n These modules can open a socket without resolving and classifying the\n' +

0 commit comments

Comments
 (0)