From 505291d9543056fd9fe227eaaac783bc0152feaa Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Mar 2026 23:16:28 +0000 Subject: [PATCH] [AI] Fix SSRF vulnerability in SimpleFIN bank sync integration Add comprehensive SSRF protection to SimpleFIN endpoints and harden the CORS proxy with consistent validation: - Create shared validate-url.ts utility with DNS-aware IP validation, TOCTOU-safe DNS pinning, and pinned HTTP fetch - Validate all SimpleFIN URLs (getAccessKey, getAccounts) against private/internal IP ranges including IPv4-mapped IPv6 - Replace naive fetch() calls with pinned DNS agents that prevent DNS rebinding between validation and request time - Add manual redirect loops with per-hop SSRF validation and cross-origin credential stripping - Enforce HTTPS protocol, request timeouts, and header hygiene - Restrict SimpleFIN access key hosts to simplefin.com/simplefin.org - Apply same IP-pinning and redirect validation to CORS proxy https://claude.ai/code/session_0122tzRmFfs3ieXaTK7msUhw --- packages/sync-server/src/app-cors-proxy.js | 101 +++++-- .../sync-server/src/app-cors-proxy.test.js | 182 ++++++------ .../src/app-simplefin/app-simplefin.js | 105 +++++-- packages/sync-server/src/util/validate-url.ts | 266 ++++++++++++++++++ upcoming-release-notes/7154.md | 6 + 5 files changed, 527 insertions(+), 133 deletions(-) create mode 100644 packages/sync-server/src/util/validate-url.ts create mode 100644 upcoming-release-notes/7154.md diff --git a/packages/sync-server/src/app-cors-proxy.js b/packages/sync-server/src/app-cors-proxy.js index c5f45c3666..411150a321 100644 --- a/packages/sync-server/src/app-cors-proxy.js +++ b/packages/sync-server/src/app-cors-proxy.js @@ -1,9 +1,9 @@ import express from 'express'; import rateLimit from 'express-rate-limit'; -import ipaddr from 'ipaddr.js'; import { config } from './load-config'; import { requestLoggerMiddleware } from './util/middlewares'; +import { isPrivateHost, pinnedFetch, validateUrl } from './util/validate-url'; import { validateSession } from './util/validate-user'; const app = express(); @@ -61,27 +61,24 @@ async function fetchAllowlist() { /** * Return true only if the URL is on an allowlist and not a local/private address. + * When isRedirect is true, also allows known GitHub CDN hosts that serve + * release assets (these use opaque signed URLs without repo path structure). */ -function isUrlAllowed(targetUrl) { +async function isUrlAllowed(targetUrl, isRedirect = false) { try { const url = new URL(targetUrl); const hostname = url.hostname; - // Block private/local IP addresses - if (ipaddr.isValid(hostname)) { - const ip = ipaddr.parse(hostname); - if ( - [ - 'private', - 'loopback', - 'linkLocal', - 'uniqueLocal', - 'unspecified', - ].includes(ip.range()) - ) { - console.warn(`Blocked request to private/localhost IP: ${hostname}`); - return false; - } + // Enforce HTTPS protocol + if (url.protocol !== 'https:') { + console.warn(`Blocked non-HTTPS request: ${url.protocol}//${hostname}`); + return false; + } + + // Block private/local IP addresses (DNS-aware, resolves hostnames) + if (await isPrivateHost(hostname)) { + console.warn(`Blocked request to private/localhost address: ${hostname}`); + return false; } // Always allow the specific plugin-store URL @@ -119,6 +116,11 @@ function isUrlAllowed(targetUrl) { } } + // Allow known GitHub CDN hosts for redirect targets (release asset downloads) + if (isRedirect && hostname.endsWith('.githubusercontent.com')) { + return true; + } + return false; } catch (e) { console.warn('Invalid target URL:', targetUrl, e.message); @@ -167,7 +169,7 @@ app.use('/', async (req, res) => { } // Check if the URL is allowed - if (!isUrlAllowed(url.href)) { + if (!(await isUrlAllowed(url.href))) { console.warn('Blocked request to unauthorized URL:', url.href); return res.status(403).json({ error: 'URL not allowed', @@ -193,7 +195,6 @@ app.use('/', async (req, res) => { const requestHeaders = { ...req.headers, ...customHeaders, - host: url.host, }; // Remove headers that shouldn't be forwarded @@ -201,6 +202,8 @@ app.use('/', async (req, res) => { delete requestHeaders['content-length']; delete requestHeaders['cookie']; delete requestHeaders['cookie2']; + delete requestHeaders['accept-encoding']; + delete requestHeaders['host']; // Add GitHub authentication if token is configured and request is to GitHub const githubToken = config.get('github.token'); @@ -217,15 +220,57 @@ app.use('/', async (req, res) => { ); } - const response = await fetch(url.href, { - method, - headers: requestHeaders, - body: ['GET', 'HEAD'].includes(method) - ? undefined - : typeof body === 'string' - ? body - : JSON.stringify(body), - }); + // Pinned fetch with per-hop redirect validation + const MAX_REDIRECTS = 5; + let currentUrl = url.href; + const originalOrigin = url.origin; + let currentHeaders = { ...requestHeaders }; + let response; + + for (let i = 0; i <= MAX_REDIRECTS; i++) { + const { resolvedAddress, family } = await validateUrl(currentUrl); + + response = await pinnedFetch(currentUrl, resolvedAddress, family, { + method: methodNormalized, + headers: currentHeaders, + }); + + if (response.status >= 300 && response.status < 400) { + const location = response.headers.get('location'); + if (!location) { + break; + } + const nextUrl = new URL(location, currentUrl); + + // Enforce HTTPS on redirect hops + if (nextUrl.protocol !== 'https:') { + return res + .status(502) + .json({ error: 'Redirect to non-HTTPS URL is not allowed' }); + } + + // Re-check allowlist for redirect target + if (!(await isUrlAllowed(nextUrl.href, true))) { + return res + .status(403) + .json({ error: 'Redirect target is not on the allowlist' }); + } + + currentUrl = nextUrl.toString(); + + // Strip auth headers on cross-origin redirects + if (nextUrl.origin !== originalOrigin) { + currentHeaders = { ...currentHeaders }; + delete currentHeaders['Authorization']; + } + + if (i === MAX_REDIRECTS) { + return res.status(502).json({ error: 'Too many redirects' }); + } + continue; + } + break; + } const contentType = response.headers.get('content-type') || 'application/octet-stream'; diff --git a/packages/sync-server/src/app-cors-proxy.test.js b/packages/sync-server/src/app-cors-proxy.test.js index ef39e98893..f8663efd4a 100644 --- a/packages/sync-server/src/app-cors-proxy.test.js +++ b/packages/sync-server/src/app-cors-proxy.test.js @@ -1,9 +1,9 @@ -import ipaddr from 'ipaddr.js'; import request from 'supertest'; import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; import { handlers as app, clearAllowlistCache } from './app-cors-proxy'; import { config } from './load-config'; +import { isPrivateHost, pinnedFetch, validateUrl } from './util/validate-url'; import { validateSession } from './util/validate-user'; vi.mock('./load-config', () => ({ @@ -24,11 +24,14 @@ vi.mock('express-rate-limit', () => ({ default: vi.fn(() => (req, res, next) => next()), })); -vi.mock('ipaddr.js', () => ({ - default: { - isValid: vi.fn().mockReturnValue(false), - parse: vi.fn(), - }, +vi.mock('./util/validate-url', () => ({ + isPrivateHost: vi.fn().mockResolvedValue(false), + validateUrl: vi.fn().mockResolvedValue({ + url: new URL('https://example.com'), + resolvedAddress: '1.2.3.4', + family: 4, + }), + pinnedFetch: vi.fn(), })); global.fetch = vi.fn(); @@ -39,15 +42,14 @@ describe('app-cors-proxy', () => { 'https://github.com/user/repo2', ]; - const createFetchMock = (options = {}) => { + const createAllowlistFetchMock = (options = {}) => { const { allowlistedRepos = defaultAllowlistedRepos, allowlistFetchFails = false, allowlistHttpError = false, - proxyResponses = {}, } = options; - return vi.fn().mockImplementation((url, _requestOptions) => { + return vi.fn().mockImplementation(url => { if ( url === 'https://raw.githubusercontent.com/actualbudget/plugin-store/refs/heads/main/plugins.json' @@ -70,104 +72,108 @@ describe('app-cors-proxy', () => { }); } - if (proxyResponses[url]) { - const response = proxyResponses[url]; - if (response.error) { - return Promise.reject(response.error); - } - return Promise.resolve(response); - } - return Promise.resolve({ ok: true, - text: () => Promise.resolve('default response'), - headers: { - get: () => 'text/plain', - }, - status: 200, + json: () => Promise.resolve([]), }); }); }; - const comprehensiveProxyResponses = { + const proxyResponses = { 'https://github.com/user/repo1': { - ok: true, - text: () => Promise.resolve('test content'), - headers: { get: () => 'text/plain' }, status: 200, + text: () => Promise.resolve('test content'), + arrayBuffer: () => Promise.resolve(new ArrayBuffer(0)), + headers: { get: () => 'text/plain' }, }, 'https://api.github.com/repos/user/repo1/releases': { - ok: true, - text: () => Promise.resolve('{"name": "test"}'), - headers: { get: () => 'application/json' }, status: 200, + text: () => Promise.resolve('{"name": "test"}'), + arrayBuffer: () => Promise.resolve(new ArrayBuffer(0)), + headers: { get: () => 'application/json' }, }, 'https://api.github.com/repos/user/repo1': { - ok: true, - text: () => Promise.resolve('{"name": "repo1"}'), - headers: { get: () => 'application/json' }, status: 200, + text: () => Promise.resolve('{"name": "repo1"}'), + arrayBuffer: () => Promise.resolve(new ArrayBuffer(0)), + headers: { get: () => 'application/json' }, }, 'https://raw.githubusercontent.com/user/repo1/main/file.txt': { - ok: true, - text: () => Promise.resolve('file content'), - headers: { get: () => 'text/plain' }, status: 200, + text: () => Promise.resolve('file content'), + arrayBuffer: () => Promise.resolve(new ArrayBuffer(0)), + headers: { get: () => 'text/plain' }, }, 'https://github.com/user/repo1/releases/download/v1.0.0/file.zip': { - ok: true, + status: 200, + text: () => Promise.resolve(''), arrayBuffer: () => Promise.resolve(new TextEncoder().encode('release content').buffer), headers: { get: () => 'application/octet-stream' }, - status: 200, }, 'https://github.com/user/repo1/manifest.json': { - ok: true, + status: 200, text: () => Promise.resolve(JSON.stringify({ name: 'test', version: '1.0.0' })), + arrayBuffer: () => Promise.resolve(new ArrayBuffer(0)), headers: { get: () => 'application/json' }, - status: 200, }, 'https://github.com/user/repo1/package.json': { - ok: true, - text: () => Promise.resolve(JSON.stringify({ test: true })), - headers: { get: () => 'text/plain' }, status: 200, + text: () => Promise.resolve(JSON.stringify({ test: true })), + arrayBuffer: () => Promise.resolve(new ArrayBuffer(0)), + headers: { get: () => 'text/plain' }, }, 'https://github.com/user/repo1/readme.txt': { - ok: true, - text: () => Promise.resolve('Hello, world!'), - headers: { get: () => 'text/plain' }, status: 200, + text: () => Promise.resolve('Hello, world!'), + arrayBuffer: () => Promise.resolve(new ArrayBuffer(0)), + headers: { get: () => 'text/plain' }, }, 'https://github.com/user/repo1/file.bin': { - ok: true, + status: 200, + text: () => Promise.resolve(''), arrayBuffer: () => Promise.resolve(new Uint8Array([1, 2, 3, 4, 5]).buffer), headers: { get: () => 'application/octet-stream' }, - status: 200, }, 'https://github.com/user/repo1/invalid.json': { - ok: true, - text: () => Promise.resolve('not valid json'), - headers: { get: () => 'text/plain' }, status: 200, - }, - 'https://github.com/user/repo1/network-error': { - error: new Error('Network error'), + text: () => Promise.resolve('not valid json'), + arrayBuffer: () => Promise.resolve(new ArrayBuffer(0)), + headers: { get: () => 'text/plain' }, }, }; + const defaultPinnedResponse = { + status: 200, + text: () => Promise.resolve('default response'), + arrayBuffer: () => Promise.resolve(new ArrayBuffer(0)), + headers: { get: () => 'text/plain' }, + }; + beforeAll(() => { - global.fetch = createFetchMock({ - proxyResponses: comprehensiveProxyResponses, - }); + global.fetch = createAllowlistFetchMock(); }); beforeEach(() => { validateSession.mockClear?.(); - ipaddr.isValid.mockClear?.(); - ipaddr.parse.mockClear?.(); + isPrivateHost.mockClear?.(); + isPrivateHost.mockResolvedValue(false); + validateUrl.mockClear?.(); + validateUrl.mockResolvedValue({ + url: new URL('https://example.com'), + resolvedAddress: '1.2.3.4', + family: 4, + }); + pinnedFetch.mockClear?.(); + pinnedFetch.mockImplementation(url => { + const response = proxyResponses[url]; + if (response) { + return Promise.resolve(response); + } + return Promise.resolve(defaultPinnedResponse); + }); validateSession.mockReturnValue({ userId: 'test-user' }); @@ -237,32 +243,38 @@ describe('app-cors-proxy', () => { validateSession.mockReturnValue({ userId: 'test-user' }); }); - it('should block private IP addresses', async () => { - ipaddr.isValid.mockReturnValueOnce(true); - ipaddr.parse.mockReturnValueOnce({ - range: () => 'private', - }); - + it('should block non-HTTPS URLs', async () => { const res = await request(app) .get('/') - .query({ url: 'http://192.168.1.1/test' }); + .query({ url: 'http://github.com/user/repo1' }); expect(res.statusCode).toBe(403); expect(res.body.error).toBe('URL not allowed'); expect(console.warn).toHaveBeenCalledWith( - 'Blocked request to private/localhost IP: 192.168.1.1', + 'Blocked non-HTTPS request: http://github.com', + ); + }); + + it('should block private IP addresses', async () => { + isPrivateHost.mockResolvedValueOnce(true); + + const res = await request(app) + .get('/') + .query({ url: 'https://192.168.1.1/test' }); + + expect(res.statusCode).toBe(403); + expect(res.body.error).toBe('URL not allowed'); + expect(console.warn).toHaveBeenCalledWith( + 'Blocked request to private/localhost address: 192.168.1.1', ); }); it('should block loopback addresses', async () => { - ipaddr.isValid.mockReturnValueOnce(true); - ipaddr.parse.mockReturnValueOnce({ - range: () => 'loopback', - }); + isPrivateHost.mockResolvedValueOnce(true); const res = await request(app) .get('/') - .query({ url: 'http://127.0.0.1/test' }); + .query({ url: 'https://127.0.0.1/test' }); expect(res.statusCode).toBe(403); expect(res.body.error).toBe('URL not allowed'); @@ -330,9 +342,8 @@ describe('app-cors-proxy', () => { }); it('should handle allowlist fetch failure gracefully', async () => { - global.fetch = createFetchMock({ + global.fetch = createAllowlistFetchMock({ allowlistFetchFails: true, - proxyResponses: comprehensiveProxyResponses, }); const res = await request(app) @@ -345,15 +356,12 @@ describe('app-cors-proxy', () => { expect.any(Error), ); - global.fetch = createFetchMock({ - proxyResponses: comprehensiveProxyResponses, - }); + global.fetch = createAllowlistFetchMock(); }); it('should handle allowlist fetch HTTP error', async () => { - global.fetch = createFetchMock({ + global.fetch = createAllowlistFetchMock({ allowlistHttpError: true, - proxyResponses: comprehensiveProxyResponses, }); const res = await request(app) @@ -366,9 +374,7 @@ describe('app-cors-proxy', () => { expect.any(Error), ); - global.fetch = createFetchMock({ - proxyResponses: comprehensiveProxyResponses, - }); + global.fetch = createAllowlistFetchMock(); }); }); @@ -428,8 +434,10 @@ describe('app-cors-proxy', () => { .get('/') .query({ url: 'https://api.github.com/repos/user/repo1' }); - expect(global.fetch).toHaveBeenCalledWith( + expect(pinnedFetch).toHaveBeenCalledWith( 'https://api.github.com/repos/user/repo1', + '1.2.3.4', + 4, expect.objectContaining({ headers: expect.objectContaining({ Authorization: 'Bearer test-github-token', @@ -446,8 +454,10 @@ describe('app-cors-proxy', () => { url: 'https://raw.githubusercontent.com/user/repo1/main/file.txt', }); - expect(global.fetch).toHaveBeenCalledWith( + expect(pinnedFetch).toHaveBeenCalledWith( 'https://raw.githubusercontent.com/user/repo1/main/file.txt', + '1.2.3.4', + 4, expect.objectContaining({ headers: expect.objectContaining({ Authorization: 'Bearer test-github-token', @@ -464,8 +474,10 @@ describe('app-cors-proxy', () => { .get('/') .query({ url: 'https://api.github.com/repos/user/repo1' }); - expect(global.fetch).toHaveBeenCalledWith( + expect(pinnedFetch).toHaveBeenCalledWith( 'https://api.github.com/repos/user/repo1', + '1.2.3.4', + 4, expect.objectContaining({ headers: expect.not.objectContaining({ Authorization: expect.any(String), @@ -546,6 +558,8 @@ describe('app-cors-proxy', () => { }); it('should handle fetch errors', async () => { + pinnedFetch.mockRejectedValueOnce(new Error('Network error')); + const res = await request(app) .get('/') .query({ url: 'https://github.com/user/repo1/network-error' }); diff --git a/packages/sync-server/src/app-simplefin/app-simplefin.js b/packages/sync-server/src/app-simplefin/app-simplefin.js index a35e54f82a..b6c0f93b8b 100644 --- a/packages/sync-server/src/app-simplefin/app-simplefin.js +++ b/packages/sync-server/src/app-simplefin/app-simplefin.js @@ -1,5 +1,3 @@ -import https from 'https'; - import express from 'express'; import { handleError } from '../app-gocardless/util/handle-error'; @@ -8,6 +6,7 @@ import { requestLoggerMiddleware, validateSessionMiddleware, } from '../util/middlewares'; +import { pinnedFetch, validateUrl } from '../util/validate-url'; const app = express(); export { app as handlers }; @@ -305,6 +304,34 @@ function parseAccessKey(accessKey) { [auth, rest] = rest.split('@'); [username, password] = auth.split(':'); baseUrl = `${scheme}//${rest}`; + + // Validate that the parsed base URL points to an expected SimpleFIN endpoint + let parsedUrl; + try { + parsedUrl = new URL(baseUrl); + } catch (e) { + console.log('Invalid SimpleFIN base URL in access key'); + throw new Error('Invalid access key'); + } + + // Enforce HTTPS and restrict the hostname to the SimpleFIN domain + if (parsedUrl.protocol !== 'https:') { + console.log('Invalid SimpleFIN access key protocol:', parsedUrl.protocol); + throw new Error('Invalid access key'); + } + + const hostname = parsedUrl.hostname.toLowerCase(); + const isAllowedHost = + hostname === 'simplefin.com' || + hostname.endsWith('.simplefin.com') || + hostname === 'simplefin.org' || + hostname.endsWith('.simplefin.org'); + + if (!isAllowedHost) { + console.log('Invalid SimpleFIN access key host:', hostname); + throw new Error('Invalid access key'); + } + return { baseUrl, username, @@ -314,22 +341,22 @@ function parseAccessKey(accessKey) { async function getAccessKey(base64Token) { const token = Buffer.from(base64Token, 'base64').toString(); - const options = { + const tokenUrl = new URL(token); + if (tokenUrl.protocol !== 'https:') { + throw new Error('Setup token must use HTTPS'); + } + const { resolvedAddress, family } = await validateUrl(token); + + const response = await pinnedFetch(token, resolvedAddress, family, { method: 'POST', - port: 443, - headers: { 'Content-Length': 0 }, - }; - return new Promise((resolve, reject) => { - const req = https.request(new URL(token), options, res => { - res.on('data', d => { - resolve(d.toString()); - }); - }); - req.on('error', e => { - reject(e); - }); - req.end(); + headers: { 'Content-Length': '0' }, }); + + if (response.status < 200 || response.status >= 300) { + throw new Error(`getAccessKey failed with status ${response.status}`); + } + + return response.text(); } async function getTransactions(accessKey, accounts, startDate, endDate) { @@ -385,11 +412,47 @@ async function getAccounts( const url = new URL(`${sfin.baseUrl}/accounts`); url.search = params.toString(); - const response = await fetch(url.toString(), { - method: 'GET', - headers, - redirect: 'follow', - }); + const MAX_REDIRECTS = 5; + let currentUrl = url.toString(); + const originalOrigin = url.origin; + let currentHeaders = { ...headers }; + let response; + + for (let i = 0; i <= MAX_REDIRECTS; i++) { + const { resolvedAddress, family } = await validateUrl(currentUrl); + + response = await pinnedFetch(currentUrl, resolvedAddress, family, { + method: 'GET', + headers: currentHeaders, + }); + + if (response.status >= 300 && response.status < 400) { + const location = response.headers.get('location'); + if (!location) { + throw new Error('Redirect response missing Location header'); + } + const nextUrl = new URL(location, currentUrl); + + // Reject non-HTTPS redirects + if (nextUrl.protocol !== 'https:') { + throw new Error('Insecure redirect to non-HTTPS URL'); + } + + currentUrl = nextUrl.toString(); + + // Strip Authorization header on cross-origin redirects + if (nextUrl.origin !== originalOrigin) { + currentHeaders = { ...currentHeaders }; + delete currentHeaders.Authorization; + } + + if (i === MAX_REDIRECTS) { + throw new Error('Too many redirects'); + } + continue; + } + break; + } if (response.status === 403) { throw new Error('Forbidden'); diff --git a/packages/sync-server/src/util/validate-url.ts b/packages/sync-server/src/util/validate-url.ts new file mode 100644 index 0000000000..1c9119a2d2 --- /dev/null +++ b/packages/sync-server/src/util/validate-url.ts @@ -0,0 +1,266 @@ +import dns from 'dns'; +import http from 'http'; +import https from 'https'; + +import ipaddr from 'ipaddr.js'; + +const BLOCKED_RANGES = [ + 'private', + 'loopback', + 'linkLocal', + 'uniqueLocal', + 'unspecified', + 'broadcast', + 'reserved', + 'carrierGradeNat', +] as const; + +const DEFAULT_TIMEOUT_MS = 30_000; + +function isBlockedIP(ip: string): boolean { + if (!ipaddr.isValid(ip)) { + return false; + } + const parsed = ipaddr.process(ip); + return (BLOCKED_RANGES as readonly string[]).includes(parsed.range()); +} + +/** + * Synchronous check for literal IP addresses only. + * For hostname validation with DNS resolution, use isPrivateHost(). + */ +export function isPrivateIP(hostname: string): boolean { + return isBlockedIP(hostname); +} + +function isErrnoException(err: unknown): err is NodeJS.ErrnoException { + return err instanceof Error && 'code' in err; +} + +async function safeResolve( + resolver: (hostname: string) => Promise, + hostname: string, +): Promise { + try { + return await resolver(hostname); + } catch (err: unknown) { + if (isErrnoException(err)) { + const { code } = err; + if (code === 'ENODATA' || code === 'ENOTFOUND') { + return []; + } + } + throw err; + } +} + +/** + * Async DNS-aware check that resolves hostnames and validates + * all resolved addresses (both IPv4 and IPv6) against blocked IP ranges. + */ +export async function isPrivateHost(hostname: string): Promise { + if (ipaddr.isValid(hostname)) { + return isBlockedIP(hostname); + } + try { + const [ipv4Results, ipv6Results] = await Promise.all([ + safeResolve(dns.promises.resolve4, hostname), + safeResolve(dns.promises.resolve6, hostname), + ]); + const allAddresses = [...ipv4Results, ...ipv6Results]; + if (allAddresses.length === 0) { + // No DNS records found at all — block to be safe + return true; + } + return allAddresses.some(address => isBlockedIP(address)); + } catch { + // If DNS resolution fails, block the request to be safe + return true; + } +} + +type ValidatedUrl = { + url: URL; + resolvedAddress: string; + family: 4 | 6; +}; + +function toAddressFamily(family: number): 4 | 6 { + if (family === 4 || family === 6) { + return family; + } + throw new Error(`Unexpected DNS address family: ${family}`); +} + +/** + * Resolves and validates a URL, returning the pinned resolved address. + * Throws if the URL resolves to a private/internal IP. + */ +export async function validateUrl(urlString: string): Promise { + const url = new URL(urlString); + const { hostname } = url; + + if (ipaddr.isValid(hostname)) { + if (isBlockedIP(hostname)) { + throw new Error( + `Request to private/internal IP address is not allowed: ${hostname}`, + ); + } + return { + url, + resolvedAddress: hostname, + family: ipaddr.parse(hostname).kind() === 'ipv6' ? 6 : 4, + }; + } + + const results = await dns.promises.lookup(hostname, { all: true }); + for (const entry of results) { + if (isBlockedIP(entry.address)) { + throw new Error( + `Request to hostname resolving to private/internal IP is not allowed: ${hostname} -> ${entry.address}`, + ); + } + } + const first = results[0]; + return { + url, + resolvedAddress: first.address, + family: toAddressFamily(first.family), + }; +} + +type PinnedLookup = ( + _hostname: string, + _options: dns.LookupOptions, + callback: ( + err: NodeJS.ErrnoException | null, + address: string, + family: number, + ) => void, +) => void; + +function createPinnedLookup( + resolvedAddress: string, + family: 4 | 6, +): PinnedLookup { + return (_hostname, _options, callback) => { + callback(null, resolvedAddress, family); + }; +} + +type PinnedFetchOptions = { + method?: string; + headers?: Record; + timeout?: number; +}; + +/** + * Performs an HTTP/HTTPS request using a pinned resolved address to prevent + * TOCTOU DNS attacks. Works like fetch() but uses http/https.request with + * a custom agent that forces the pre-validated IP. + */ +export type PinnedFetchResponse = { + status: number; + headers: { get(name: string): string | null }; + text(): Promise; + arrayBuffer(): Promise; +}; + +export function pinnedFetch( + urlString: string, + resolvedAddress: string, + family: 4 | 6, + options: PinnedFetchOptions = {}, +): Promise { + const url = new URL(urlString); + + // Enforce safe protocols + if (url.protocol !== 'http:' && url.protocol !== 'https:') { + throw new Error(`Unsupported protocol in URL: ${url.protocol}`); + } + + // Block direct access to localhost by hostname + const hostname = url.hostname.toLowerCase(); + if ( + hostname === 'localhost' || + hostname === '127.0.0.1' || + hostname === '::1' + ) { + throw new Error('Requests to localhost are not allowed'); + } + + const isHttps = url.protocol === 'https:'; + const mod = isHttps ? https : http; + const lookup = createPinnedLookup(resolvedAddress, family); + const timeoutMs = options.timeout ?? DEFAULT_TIMEOUT_MS; + + const agent = isHttps + ? new https.Agent({ lookup }) + : new http.Agent({ lookup }); + + return new Promise((resolve, reject) => { + let settled = false; + + function fail(err: Error) { + if (settled) return; + settled = true; + req.destroy(); + agent.destroy(); + reject(err); + } + + const req = mod.request( + url, + { + method: options.method || 'GET', + headers: options.headers || {}, + agent, + timeout: timeoutMs, + }, + res => { + res.setTimeout(timeoutMs, () => { + fail(new Error(`Socket timeout after ${timeoutMs}ms`)); + }); + const responseHeaders = res.headers; + const chunks: Buffer[] = []; + res.on('data', (chunk: Buffer) => chunks.push(chunk)); + res.on('end', () => { + settled = true; + agent.destroy(); + const buf = Buffer.concat(chunks); + resolve({ + status: res.statusCode || 0, + headers: { + get(name: string): string | null { + const val = responseHeaders[name.toLowerCase()]; + if (val == null) return null; + return Array.isArray(val) ? val[0] : val; + }, + }, + text: () => Promise.resolve(buf.toString()), + arrayBuffer: () => + Promise.resolve( + buf.buffer.slice( + buf.byteOffset, + buf.byteOffset + buf.byteLength, + ), + ), + }); + }); + res.on('error', (err: Error) => fail(err)); + }, + ); + req.on('timeout', () => { + fail(new Error(`Request timeout after ${timeoutMs}ms`)); + }); + req.on('error', (err: Error) => fail(err)); + req.end(); + }); +} + +/** + * Backward-compatible wrapper that validates and throws on private URLs. + */ +export async function assertNotPrivateUrl(urlString: string): Promise { + await validateUrl(urlString); +} diff --git a/upcoming-release-notes/7154.md b/upcoming-release-notes/7154.md new file mode 100644 index 0000000000..2a608eaee5 --- /dev/null +++ b/upcoming-release-notes/7154.md @@ -0,0 +1,6 @@ +--- +category: Bugfix +authors: [MatissJanis] +--- + +Add URL validation to block requests to private IP addresses, enhancing security measures.