From 47071264239875c7ceb84f85fa23207333be71c1 Mon Sep 17 00:00:00 2001 From: SandObserver <260779319+SandObserver@users.noreply.github.com> Date: Sat, 17 Oct 2026 20:44:37 -0400 Subject: [PATCH] Refuse loopback aliases and more IPv6 ranges in the SSRF guard Resolve dotless service names in guardSsrf and pin the result. Refuse a name that resolves to loopback, unspecified or link-local, which covers ip6-localhost, ip6-loopback and a host name mapped to 127.0.1.1. Private addresses stay allowed for Docker service names. Block 6to4 (2002::/16), local-use NAT64 (64:ff9b:1::/48) and site-local (fec0::/10) IPv6 literals and resolved addresses. --- api/src/proxy.js | 50 ++++++++++++++++++++--------- api/test/blocked-ranges.test.js | 7 ++++ api/test/proxy-ping-rewrite.test.js | 24 +++++++------- api/test/proxy-rewrite.test.js | 11 ++++--- api/test/proxy.test.js | 25 +++++++++++++-- api/test/url-scheme-policy.test.js | 4 ++- 6 files changed, 86 insertions(+), 35 deletions(-) diff --git a/api/src/proxy.js b/api/src/proxy.js index c68a2de5..cc4e3589 100644 --- a/api/src/proxy.js +++ b/api/src/proxy.js @@ -27,7 +27,12 @@ const BLOCKED_IPV4 = [ /* ff00::/8 needs all four hex digits. A group written 'ff' is 0x00ff, which is not multicast. */ -const BLOCKED_IPV6_RE = /^(::1$|::$|f[cd][0-9a-f]{2}:|fe[89ab][0-9a-f]:|ff[0-9a-f]{2}:)/i; +const BLOCKED_IPV6_RE = /^(::1$|::$|f[cd][0-9a-f]{2}:|fe[89a-f][0-9a-f]:|ff[0-9a-f]{2}:|2002:|64:ff9b:1:)/i; + +/* Refused for a Docker service name too. These reach this machine or the cloud + metadata service, never another container. */ +const SELF_IPV4 = new Set(['0.0.0.0', '127.0.0.0', '169.254.0.0']); +const SELF_IPV6_RE = /^(::1$|::$|fe[89ab][0-9a-f]:)/i; /** @param {string} addr @returns {number|null} */ function ipv4ToInt(addr) { @@ -36,19 +41,27 @@ function ipv4ToInt(addr) { return ((+p[0] << 24) >>> 0) + (+p[1] << 16) + (+p[2] << 8) + +p[3]; } -const _blockedV4 = BLOCKED_IPV4.map(([base, bits]) => ({ - /* A /0 would need a 32-bit shift, which JavaScript treats as a no-op. */ - mask: bits === 0 ? 0 : (0xffffffff << (32 - bits)) >>> 0, - base: ipv4ToInt(base), -})); +/** @param {Array<[string, number, string]>} list */ +const toRanges = list => + list.map(([base, bits]) => ({ + /* A /0 would need a 32-bit shift, which JavaScript treats as a no-op. */ + mask: bits === 0 ? 0 : (0xffffffff << (32 - bits)) >>> 0, + base: ipv4ToInt(base), + })); -/** @param {string} addr @returns {boolean} */ -function isBlockedIPv4(addr) { +const _blockedV4 = toRanges(BLOCKED_IPV4); +const _selfV4 = toRanges(BLOCKED_IPV4.filter(([base]) => SELF_IPV4.has(base))); + +/** @param {string} addr @param {Array<{ mask: number, base: number | null }>} ranges */ +function inRanges(addr, ranges) { const n = ipv4ToInt(addr); if (n === null) return false; - return _blockedV4.some(r => (n & r.mask) >>> 0 === r.base); + return ranges.some(r => (n & r.mask) >>> 0 === r.base); } +/** @param {string} addr @returns {boolean} */ +const isBlockedIPv4 = addr => inRanges(addr, _blockedV4); + /* The range check only understands dotted-decimal. Without this a hex-tailed literal such as ::7f00:1 or 64:ff9b::a9fe:a9fe slips past it. */ function embeddedIPv4(addr) { @@ -66,20 +79,26 @@ function embeddedIPv4(addr) { return `${hi >> 8}.${hi & 0xff}.${lo >> 8}.${lo & 0xff}`; } -function isPrivateAddress(addr) { +function inAddressSet(addr, v4Ranges, v6Re) { if (typeof addr !== 'string' || !addr) return false; const s = addr.toLowerCase(); if (s.startsWith('::ffff:') || s.startsWith('64:ff9b::')) { const v4 = embeddedIPv4(s); - if (v4) return isBlockedIPv4(v4); + if (v4) return inRanges(v4, v4Ranges); return true; /* unparseable tail behind a wrapper prefix: refuse */ } if (s.startsWith('::')) { const v4 = embeddedIPv4(s); - if (v4) return isBlockedIPv4(v4); + if (v4) return inRanges(v4, v4Ranges); } - return isBlockedIPv4(s) || BLOCKED_IPV6_RE.test(s); + return inRanges(s, v4Ranges) || v6Re.test(s); } + +/** @param {string} addr @returns {boolean} */ +const isPrivateAddress = addr => inAddressSet(addr, _blockedV4, BLOCKED_IPV6_RE); + +/** Loopback, unspecified or link-local. @param {string} addr @returns {boolean} */ +const isSelfAddress = addr => inAddressSet(addr, _selfV4, SELF_IPV6_RE); /* Never sniff this from the body alone. A plain-text line like "Version 1.2" matches the metric grammar exactly. */ function looksLikeMetrics(ct, body) { @@ -219,10 +238,8 @@ async function guardSsrf(rawUrl, ms = FETCH_MS) { const policy = urlPolicyError(u); if (policy) return { error: policy, ip: null }; const h = bareHost(u.hostname); - /* Loopback, not a service name. Block it before the allowance below. */ if (!ALLOW_PRIVATE_IPS && h === 'localhost') return { error: `Blocked: ${h} is a private address.`, ip: null, reason: PRIVATE_ADDRESS }; - if (isDockerServiceName(h)) return { error: null, ip: null }; const hostIp = getHostIp(); if (hostIp && h === hostIp) return { error: null, ip: null }; if (!ALLOW_PRIVATE_IPS && isPrivateAddress(h)) @@ -234,7 +251,8 @@ async function guardSsrf(rawUrl, ms = FETCH_MS) { if (e instanceof Error && e.message === 'Timed out') throw e; return { error: `Blocked: ${h} could not be resolved.`, ip: null, reason: UNRESOLVED }; } - if (!ALLOW_PRIVATE_IPS && isPrivateAddress(address)) + const refused = isDockerServiceName(h) ? isSelfAddress(address) : isPrivateAddress(address); + if (!ALLOW_PRIVATE_IPS && refused) return { error: `Blocked: ${h} resolves to private IP ${address}.`, ip: null, reason: PRIVATE_ADDRESS }; /* A literal address cannot be rebound. */ if (h === address) return { error: null, ip: null }; diff --git a/api/test/blocked-ranges.test.js b/api/test/blocked-ranges.test.js index 3cb0b87c..7a5ba15d 100644 --- a/api/test/blocked-ranges.test.js +++ b/api/test/blocked-ranges.test.js @@ -99,6 +99,13 @@ test('the existing IPv6 ranges still work', () => { for (const a of ['2001:db8::1', '2606:4700::1111']) assert.ok(!isPrivateAddress(a), `${a} should be allowed`); }); +test('6to4, local-use NAT64 and site-local IPv6 are blocked', () => { + for (const a of ['2002:c0a8:101::1', '2002::', '64:ff9b:1::a00:1', '64:ff9b:1:ffff::1', 'fec0::1', 'feff::1']) + assert.ok(isPrivateAddress(a), `${a} should be blocked`); + for (const a of ['2003::1', '64:ff9b:2::1', '64:ff9b:1a::1', 'ff::1', 'fe7f::1']) + assert.ok(!isPrivateAddress(a), `${a} should be allowed`); +}); + /* ── the new ranges through the IPv4-in-IPv6 wrappers ─────────────────────── */ /* The wrappers decode to IPv4 and then use the same table, so a range added to diff --git a/api/test/proxy-ping-rewrite.test.js b/api/test/proxy-ping-rewrite.test.js index ba75e561..cc7bcec5 100644 --- a/api/test/proxy-ping-rewrite.test.js +++ b/api/test/proxy-ping-rewrite.test.js @@ -25,6 +25,7 @@ const dns = require('node:dns'); const net = require('node:net'); const log = require('../src/log'); const { fetchChecked, pingChecked, pingUnchecked, SsrfBlockedError } = require('../src/proxy'); +const { errorBody } = require('../src/api-error'); /* Capture what proxy.js logs about the attempt, which names the host it tried. */ async function targetOf(fn) { @@ -60,13 +61,13 @@ before(async () => { }); test('pingChecked follows portMap to the mapped container', async () => { - let r; - const target = await targetOf(async () => { - r = await pingChecked(MAPPED, MS, false); - }); - assert.equal(r.ok, false); - assert.match(target, /stackyard-test-nx-host/, 'ping must target the rewritten host'); - assert.doesNotMatch(r.error, /stackyard-test-nx-host/, 'and must not tell the browser the host'); + const e = await pingChecked(MAPPED, MS, false).then( + () => null, + x => x, + ); + assert.ok(e instanceof SsrfBlockedError); + assert.match(e.message, /stackyard-test-nx-host/, 'ping must guard the rewritten host'); + assert.doesNotMatch(errorBody(e).error, /stackyard-test-nx-host/, 'and must not tell the browser the host'); }); test('pingUnchecked follows portMap to the mapped container', async () => { @@ -80,14 +81,15 @@ test('pingUnchecked follows portMap to the mapped container', async () => { }); test('ping and fetch resolve the same url to the same target', async () => { - const pingTarget = await targetOf(async () => { - await pingChecked(MAPPED, MS, false); - }); + const pingErr = await pingChecked(MAPPED, MS, false).then( + () => null, + e => e.message, + ); const fetchErr = await fetchChecked(MAPPED, { timeout: MS }).then( () => null, e => e.message, ); - assert.match(pingTarget, /stackyard-test-nx-host/); + assert.match(pingErr, /stackyard-test-nx-host/); assert.match(fetchErr, /stackyard-test-nx-host/, 'both must resolve to the same host'); }); diff --git a/api/test/proxy-rewrite.test.js b/api/test/proxy-rewrite.test.js index f10dba87..035f95bc 100644 --- a/api/test/proxy-rewrite.test.js +++ b/api/test/proxy-rewrite.test.js @@ -27,6 +27,7 @@ fs.writeFileSync( const { test } = require('node:test'); const assert = require('node:assert/strict'); +const dns = require('node:dns').promises; const { rewriteUrl, _internals } = require('../src/proxy'); const { guardSsrf } = _internals; @@ -88,8 +89,9 @@ test('rewriting changes the host, so the guard must run after it', async () => { assert.equal((await guardSsrf(raw)).error, null); }); -test('guarding the rewritten URL also passes, via the dotless-name branch', async () => { - assert.deepEqual(await guardSsrf(rewriteUrl('http://192.168.1.50:8096/')), { error: null, ip: null }); +test('guarding the rewritten URL also passes, via the dotless-name branch', async t => { + t.mock.method(dns, 'lookup', async () => ({ address: '172.18.0.5', family: 4 })); + assert.deepEqual(await guardSsrf(rewriteUrl('http://192.168.1.50:8096/')), { error: null, ip: '172.18.0.5' }); }); /* ── the service-name allowance must not swallow an IPv6 literal ─────────── */ @@ -114,6 +116,7 @@ test('guardSsrf blocks an IPv4-in-IPv6 wrapper around a loopback address', async /* The other half: a real dotless name still passes, so the exclusion above is narrow rather than a blanket refusal of short hostnames. */ -test('guardSsrf still trusts an ordinary dotless container name', async () => { - assert.deepEqual(await guardSsrf('http://jellyfin:8096/'), { error: null, ip: null }); +test('guardSsrf still trusts an ordinary dotless container name', async t => { + t.mock.method(dns, 'lookup', async () => ({ address: '172.18.0.5', family: 4 })); + assert.deepEqual(await guardSsrf('http://jellyfin:8096/'), { error: null, ip: '172.18.0.5' }); }); diff --git a/api/test/proxy.test.js b/api/test/proxy.test.js index 2634c992..dc1b9210 100644 --- a/api/test/proxy.test.js +++ b/api/test/proxy.test.js @@ -6,6 +6,7 @@ process.env.CONFIG_PATH = tmpPath('apps.json'); const { test } = require('node:test'); const assert = require('node:assert/strict'); const http = require('node:http'); +const dns = require('node:dns').promises; const { getHostIp, shouldSkipTls, isPrivateAddress, embeddedIPv4, _internals } = require('../src/proxy'); /* guardSsrf/fetchJSON are private to proxy.js: routes go through the fetchChecked/fetchUnchecked boundary. Tests reach the primitives directly. */ @@ -76,8 +77,27 @@ test('guardSsrf blocks IPv4-mapped and NAT64 loopback/metadata literals', async } }); -test('guardSsrf allows dotless Docker service names without pinning', async () => { - assert.deepEqual(await guardSsrf('http://navidrome:4533'), { error: null, ip: null }); +test('guardSsrf resolves a dotless Docker service name, allows its private address and pins it', async t => { + t.mock.method(dns, 'lookup', async () => ({ address: '172.18.0.5', family: 4 })); + assert.deepEqual(await guardSsrf('http://navidrome:4533'), { error: null, ip: '172.18.0.5' }); +}); + +test('guardSsrf blocks a dotless name that resolves to loopback, link-local or this host', async t => { + for (const address of ['::1', '127.0.1.1', '169.254.169.254', '0.0.0.0', 'fe80::1', '::ffff:127.0.0.1']) { + t.mock.method(dns, 'lookup', async () => ({ address, family: address.includes(':') ? 6 : 4 })); + for (const name of ['ip6-localhost', 'ip6-loopback', 'homelab']) { + const r = await guardSsrf(`http://${name}:8080/`); + assert.equal(r.reason, 'private-address', `${name} -> ${address}`); + assert.equal(r.ip, null); + } + } +}); + +test('guardSsrf blocks a dotless name that does not resolve', async t => { + t.mock.method(dns, 'lookup', async () => { + throw Object.assign(new Error('getaddrinfo ENOTFOUND'), { code: 'ENOTFOUND' }); + }); + assert.equal((await guardSsrf('http://typo-container:8080/')).reason, 'unresolved'); }); test('guardSsrf blocks private IP literals', async () => { @@ -131,7 +151,6 @@ test('fetchJSON pins the IP and preserves the Host header', async () => { once and the validated IP is returned to be pinned for the connection; a name that resolves to a private address is blocked even though the name itself looks public (which a stale re-resolution could otherwise exploit). ── */ -const dns = require('node:dns').promises; test('guardSsrf resolves a public hostname and returns the IP to pin', async t => { t.mock.method(dns, 'lookup', async () => ({ address: '93.184.216.34', family: 4 })); diff --git a/api/test/url-scheme-policy.test.js b/api/test/url-scheme-policy.test.js index ccf76495..c23643f5 100644 --- a/api/test/url-scheme-policy.test.js +++ b/api/test/url-scheme-policy.test.js @@ -3,6 +3,7 @@ const assert = require('node:assert/strict'); const http = require('node:http'); const https = require('node:https'); +const dns = require('node:dns').promises; const { _internals, urlPolicyError, fetchUnchecked, pingUnchecked, ALLOWED_PROTOCOLS } = require('../src/proxy'); const BAD_SCHEMES = [ @@ -66,7 +67,8 @@ test('an empty hostname no longer passes as a Docker service name', async () => assert.equal(g.ip, null); }); -test('guardSsrf still allows a dotless service name over http', async () => { +test('guardSsrf still allows a dotless service name over http', async t => { + t.mock.method(dns, 'lookup', async () => ({ address: '172.18.0.5', family: 4 })); const g = await _internals.guardSsrf('http://qbittorrent:8080/api/v2'); assert.equal(g.error, null); });