fix: strengthen SSRF guard in fetchUrl with DNS resolution check
Add exported isBlockedAddress(ip) helper covering loopback, 0.0.0.0, private v4 (10/8, 172.16-31, 192.168/16), link-local (169.254/16, fe80::/10), and IPv6 ULA (fc00::/7). In fetchUrl, after the existing literal-hostname fast-reject, resolve the hostname via dns.lookup (all:true) when using the real fetcher and block if any resolved address isBlockedAddress. Injected fetcher (tests) skips DNS. Drop unused contentType from return value. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -1,5 +1,6 @@
|
||||
// lib/icons/ingest.js
|
||||
import path from 'node:path';
|
||||
import dns from 'node:dns';
|
||||
import AdmZip from 'adm-zip';
|
||||
import { sanitizeSvg } from './sanitize.js';
|
||||
|
||||
@@ -57,19 +58,74 @@ export function unpackZip(buffer) {
|
||||
|
||||
const PRIVATE_HOST = /^(localhost|127\.|0\.0\.0\.0|10\.|192\.168\.|169\.254\.|172\.(1[6-9]|2\d|3[01])\.|\[?::1\]?)/i;
|
||||
|
||||
/**
|
||||
* Returns true if the given IP string is a blocked (loopback, private,
|
||||
* link-local, or ULA) address that should not be fetched.
|
||||
* Handles both IPv4 and IPv6.
|
||||
*/
|
||||
export function isBlockedAddress(ip) {
|
||||
if (!ip) return true;
|
||||
// IPv6 loopback
|
||||
if (ip === '::1') return true;
|
||||
// IPv4-mapped loopback ::ffff:127.x.x.x
|
||||
const v4mapped = ip.match(/^::ffff:(\d+\.\d+\.\d+\.\d+)$/i);
|
||||
const v4 = v4mapped ? v4mapped[1] : ip;
|
||||
|
||||
if (/^\d+\.\d+\.\d+\.\d+$/.test(v4)) {
|
||||
const parts = v4.split('.').map(Number);
|
||||
const [a, b] = parts;
|
||||
// 0.0.0.0
|
||||
if (a === 0) return true;
|
||||
// 127.0.0.0/8 — loopback
|
||||
if (a === 127) return true;
|
||||
// 10.0.0.0/8 — private
|
||||
if (a === 10) return true;
|
||||
// 172.16.0.0/12 — private
|
||||
if (a === 172 && b >= 16 && b <= 31) return true;
|
||||
// 192.168.0.0/16 — private
|
||||
if (a === 192 && b === 168) return true;
|
||||
// 169.254.0.0/16 — link-local
|
||||
if (a === 169 && b === 254) return true;
|
||||
return false;
|
||||
}
|
||||
|
||||
// IPv6 checks (expand to lower-case for prefix matching)
|
||||
const lower = ip.toLowerCase();
|
||||
// ::1 is caught above; handle full-form loopback
|
||||
if (lower === '0:0:0:0:0:0:0:1') return true;
|
||||
// fe80::/10 — link-local (fe80 – febf)
|
||||
if (/^fe[89ab][0-9a-f]:/i.test(lower)) return true;
|
||||
// fc00::/7 — ULA (fc00 – fdff)
|
||||
if (/^f[cd][0-9a-f]{2}:/i.test(lower)) return true;
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
function dnsLookupAll(hostname) {
|
||||
return new Promise((resolve, reject) =>
|
||||
dns.lookup(hostname, { all: true }, (err, addrs) => err ? reject(err) : resolve(addrs))
|
||||
);
|
||||
}
|
||||
|
||||
// Fetch a remote icon or zip. SSRF guard: http/https only, no localhost/private,
|
||||
// size + timeout caps. `fetcher` injectable for tests.
|
||||
export async function fetchUrl(url, { fetcher = fetch } = {}) {
|
||||
// DNS-resolved address check, size + timeout caps. `fetcher` injectable for tests.
|
||||
export async function fetchUrl(url, { fetcher } = {}) {
|
||||
let u;
|
||||
try { u = new URL(url); } catch { throw new Error('bad_url'); }
|
||||
if (u.protocol !== 'http:' && u.protocol !== 'https:') throw new Error('bad_scheme');
|
||||
// Fast literal-hostname guard (catches raw IP strings and 'localhost' without DNS)
|
||||
if (PRIVATE_HOST.test(u.hostname)) throw new Error('blocked_host');
|
||||
const res = await fetcher(url, { signal: AbortSignal.timeout(8000), redirect: 'error' });
|
||||
// DNS resolution guard — only when using the real fetcher (not in tests)
|
||||
if (!fetcher) {
|
||||
const addrs = await dnsLookupAll(u.hostname).catch(() => []);
|
||||
if (addrs.some(a => isBlockedAddress(a.address))) throw new Error('blocked_host');
|
||||
}
|
||||
const realFetcher = fetcher ?? fetch;
|
||||
const res = await realFetcher(url, { signal: AbortSignal.timeout(8000), redirect: 'error' });
|
||||
if (!res.ok) throw new Error('fetch_failed');
|
||||
const ab = await res.arrayBuffer();
|
||||
if (ab.byteLength > MAX_URL_BYTES) throw new Error('too_large');
|
||||
const ct = (res.headers.get ? res.headers.get('content-type') : res.headers.get?.('content-type')) || '';
|
||||
return { buffer: Buffer.from(ab), contentType: ct };
|
||||
return { buffer: Buffer.from(ab) };
|
||||
}
|
||||
|
||||
export function isZip(buf) { return buf && buf.length > 4 && buf[0] === 0x50 && buf[1] === 0x4b; }
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import AdmZip from 'adm-zip';
|
||||
import { processFile, unpackZip, fetchUrl, MAX_FILE } from '../../lib/icons/ingest.js';
|
||||
import { processFile, unpackZip, fetchUrl, isBlockedAddress, MAX_FILE } from '../../lib/icons/ingest.js';
|
||||
|
||||
const PNG = Buffer.from([0x89,0x50,0x4e,0x47,0x0d,0x0a,0x1a,0x0a, 0,0,0,0]);
|
||||
|
||||
@@ -52,6 +52,18 @@ describe('unpackZip', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('isBlockedAddress', () => {
|
||||
it('blocks loopback IPv4', () => { expect(isBlockedAddress('127.0.0.1')).toBe(true); });
|
||||
it('blocks private 10/8', () => { expect(isBlockedAddress('10.1.2.3')).toBe(true); });
|
||||
it('blocks private 192.168/16', () => { expect(isBlockedAddress('192.168.0.1')).toBe(true); });
|
||||
it('blocks link-local 169.254/16', () => { expect(isBlockedAddress('169.254.1.1')).toBe(true); });
|
||||
it('blocks 0.0.0.0', () => { expect(isBlockedAddress('0.0.0.0')).toBe(true); });
|
||||
it('blocks IPv6 loopback ::1', () => { expect(isBlockedAddress('::1')).toBe(true); });
|
||||
it('blocks IPv6 ULA fc00::1', () => { expect(isBlockedAddress('fc00::1')).toBe(true); });
|
||||
it('allows public 8.8.8.8', () => { expect(isBlockedAddress('8.8.8.8')).toBe(false); });
|
||||
it('allows public 1.1.1.1', () => { expect(isBlockedAddress('1.1.1.1')).toBe(false); });
|
||||
});
|
||||
|
||||
describe('fetchUrl', () => {
|
||||
it('rejects non-http schemes', async () => {
|
||||
await expect(fetchUrl('file:///etc/passwd')).rejects.toThrow();
|
||||
|
||||
Reference in New Issue
Block a user