mirror of
https://github.com/nexu-io/open-design.git
synced 2026-06-01 03:14:35 +07:00
fix(security): strip trailing dot in normalizeBracketedIpv6 (FQDN SSRF bypass) (#1122)
* fix(security): strip trailing dot in normalizeBracketedIpv6 (FQDN bypass)
new URL('http://192.168.1.5./').hostname returns '192.168.1.5.' — the
trailing dot is the RFC 1034 absolute-FQDN form and resolves identically
to '192.168.1.5'. parseIpv4 fails on the dotted form, so 169.254.169.254.
slips past the metadata-service block, 192.168.1.5. slips past the LAN
block, and localhost. slips past the loopback identification.
Strip trailing dots in normalizeBracketedIpv6 so all downstream checks
(isLoopbackApiHost, isBlockedExternalApiHostname, isBlockedIpv4, IPv6
range tests) see the canonical form.
Adds 6 vitest cases covering loopback FQDN forms (localhost.,
foo.localhost., 127.0.0.1.) and SSRF FQDN bypasses (169.254.169.254.,
192.168.1.5., 10.0.0.5.).
Refs nexu-io/open-design#1119 review feedback (P2 from @lefarcen).
* test(connectionTest): tighten trailing-dot coverage per #1122 review
Two issues from #1122 review:
1. (P2 from @mrcfps + codex bot) The original `foo.localhost.` case
asserted error===undefined on validateBaseUrl, which only proves the
URL passed validation — not that the host is identified as loopback.
Replaced with direct isLoopbackApiHost(...) assertions on the actual
loopback FQDN forms (localhost., 127.0.0.1., 127.0.0.5.) so the test
exercises the loopback path the comment claims.
2. (P3 from @lefarcen) Original blocked-FQDN tests covered only 3 of 7
ranges that isBlockedIpv4 handles. Added a dedicated case per range
(0.0.0.0/8, 10/8, 100.64/10, 169.254/16, 172.16/12, 192.168/16,
multicast >=224) so future regressions in normalizeBracketedIpv6
surface against the full coverage.
* docs: drop misleading foo.localhost./endsWith claim in normalizer comment
@lefarcen review feedback: isLoopbackApiHost only accepts exact 'localhost',
'::1', loopback IPv4, and mapped loopback IPv4 — there's no subdomain or
endsWith handling, so referencing 'foo.localhost.' overstates what the
trailing-dot strip enables. Rewrite the comment to match actual call sites
(isLoopbackApiHost equality + isBlockedIpv4 numeric parse).
This commit is contained in:
parent
b6e4ae5e11
commit
377d65b7e4
2 changed files with 41 additions and 4 deletions
|
|
@ -21,9 +21,14 @@ declare const URL: {
|
|||
};
|
||||
|
||||
function normalizeBracketedIpv6(hostname: string): string {
|
||||
return hostname.startsWith('[') && hostname.endsWith(']')
|
||||
? hostname.slice(1, -1).toLowerCase()
|
||||
: hostname.toLowerCase();
|
||||
const stripped = hostname.startsWith('[') && hostname.endsWith(']')
|
||||
? hostname.slice(1, -1)
|
||||
: hostname;
|
||||
// FQDN trailing-dot form (RFC 1034) resolves identically to the dotless form,
|
||||
// so `localhost.` must normalize to `localhost` before the equality check in
|
||||
// isLoopbackApiHost — and `0.0.0.0.`, `10.0.0.1.`, etc. must normalize before
|
||||
// isBlockedIpv4 parses them. Strips one or more trailing dots.
|
||||
return stripped.toLowerCase().replace(/\.+$/, '');
|
||||
}
|
||||
|
||||
function parseIpv4(hostname: string): [number, number, number, number] | null {
|
||||
|
|
|
|||
|
|
@ -1,5 +1,8 @@
|
|||
import { describe, expect, it } from 'vitest';
|
||||
import { validateBaseUrl } from '../src/api/connectionTest';
|
||||
import {
|
||||
isLoopbackApiHost,
|
||||
validateBaseUrl,
|
||||
} from '../src/api/connectionTest';
|
||||
|
||||
describe('provider base URL validation', () => {
|
||||
it('allows public endpoints and loopback local providers', () => {
|
||||
|
|
@ -14,6 +17,15 @@ describe('provider base URL validation', () => {
|
|||
}
|
||||
});
|
||||
|
||||
it('identifies trailing-dot FQDN forms of loopback hosts as loopback', () => {
|
||||
// Direct assertion against isLoopbackApiHost — validateBaseUrl alone
|
||||
// can't distinguish "passed because loopback" from "passed because
|
||||
// not blocked", which the previous test revision conflated.
|
||||
for (const host of ['localhost.', '127.0.0.1.', '127.0.0.5.']) {
|
||||
expect(isLoopbackApiHost(host)).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it('blocks private, link-local, CGNAT, multicast, and mapped forms', () => {
|
||||
for (const baseUrl of [
|
||||
'http://0.0.0.0:11434/v1',
|
||||
|
|
@ -34,4 +46,24 @@ describe('provider base URL validation', () => {
|
|||
});
|
||||
}
|
||||
});
|
||||
|
||||
it('blocks trailing-dot FQDN bypass across every blocked IPv4 range', () => {
|
||||
// The trailing-dot strip in normalizeBracketedIpv6 must apply to
|
||||
// every range isBlockedIpv4 covers — not just the three originally
|
||||
// demonstrated. One representative case per range:
|
||||
for (const baseUrl of [
|
||||
'http://0.0.0.0.:11434/v1', // 0.0.0.0/8
|
||||
'http://10.0.0.5.:11434/v1', // 10/8
|
||||
'http://100.64.0.1.:11434/v1', // 100.64/10 CGNAT
|
||||
'http://169.254.169.254./latest/meta-data', // 169.254/16 metadata
|
||||
'http://172.16.0.5.:11434/v1', // 172.16/12
|
||||
'http://192.168.1.5.:11434/v1', // 192.168/16
|
||||
'http://224.0.0.1.:11434/v1', // multicast >=224
|
||||
]) {
|
||||
expect(validateBaseUrl(baseUrl)).toMatchObject({
|
||||
error: 'Internal IPs blocked',
|
||||
forbidden: true,
|
||||
});
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue