fix: further guard against DNS rebinding attack

This commit is contained in:
Julian Lam
2025-05-24 22:12:48 -04:00
parent 70c04f0cb2
commit a8e613e13a

View File

@@ -1,6 +1,7 @@
'use strict'; 'use strict';
const dns = require('dns').promises; const dns = require('dns').promises;
require('undici'); // keep this here, needed for SSRF (see `lookup()`)
const nconf = require('nconf'); const nconf = require('nconf');
const ipaddr = require('ipaddr.js'); const ipaddr = require('ipaddr.js');
@@ -35,9 +36,39 @@ async function init() {
initialized = true; initialized = true;
} }
/**
* This method (alongside `check()`) guards against SSRF via DNS rebinding.
*
* - `check()` does a DNS lookup and ensures that all returned IPs do not belong to a reserved IP address space
* - `lookup()` provides additional logic that uses the cached DNS result from `check()`
* instead of doing another lookup (which is where DNS rebinding comes into play.)
* - For whatever reason `undici` needs to be required so that lookup can be overwritten properly.
*/
function lookup(hostname, options, callback) {
const { ok, lookup } = checkCache.get(hostname);
if (!ok) {
throw new Error('lookup-failed');
}
if (!lookup) {
// trusted, do regular lookup
dns.lookup(hostname, options).then((addresses) => {
callback(null, addresses);
});
return;
}
if (options.all === true) {
callback(null, lookup);
} else {
const { address, family } = lookup.shift();
callback(null, address, family);
}
}
// Initialize fetch - somewhat hacky, but it's required for globalDispatcher to be available // Initialize fetch - somewhat hacky, but it's required for globalDispatcher to be available
async function call(url, method, { body, timeout, jar, ...config } = {}) { async function call(url, method, { body, timeout, jar, ...config } = {}) {
const ok = await check(url); const { ok } = await check(url);
if (!ok) { if (!ok) {
throw new Error('[[error:reserved-ip-address]]'); throw new Error('[[error:reserved-ip-address]]');
} }
@@ -75,7 +106,9 @@ async function call(url, method, { body, timeout, jar, ...config } = {}) {
return super.dispatch(opts, handler); return super.dispatch(opts, handler);
} }
} }
opts.dispatcher = new FetchAgent(); opts.dispatcher = new FetchAgent({
connect: { lookup },
});
} }
const response = await fetchImpl(url, opts); const response = await fetchImpl(url, opts);
@@ -109,33 +142,36 @@ async function check(url) {
await init(); await init();
const { host } = new URL(url); const { host } = new URL(url);
if (allowList.has(host)) {
return true;
}
const cached = checkCache.get(url); const cached = checkCache.get(url);
if (cached !== undefined) { if (cached !== undefined) {
return cached; return cached;
} }
if (allowList.has(host)) {
const payload = { ok: true };
checkCache.set(host, payload);
return payload;
}
const addresses = new Set(); const addresses = new Set();
let lookup;
if (ipaddr.isValid(url)) { if (ipaddr.isValid(url)) {
addresses.add(url); addresses.add(url);
} else { } else {
const lookup = await dns.lookup(host, { all: true }); lookup = await dns.lookup(host, { all: true });
lookup.forEach(({ address }) => { lookup.forEach(({ address, family }) => {
addresses.add(address); addresses.add({ address, family });
}); });
} }
// Every IP address that the host resolves to should be a unicast address // Every IP address that the host resolves to should be a unicast address
const ok = Array.from(addresses).every((ip) => { const ok = Array.from(addresses).every(({ address: ip }) => {
const parsed = ipaddr.parse(ip); const parsed = ipaddr.parse(ip);
return parsed.range() === 'unicast'; return parsed.range() === 'unicast';
}); });
checkCache.set(host, ok); const payload = { ok, lookup };
return ok; checkCache.set(host, payload);
return payload;
} }
/* /*