diff --git a/backend/__tests__/utils/networkValidation.dns.test.js b/backend/__tests__/utils/networkValidation.dns.test.js new file mode 100644 index 00000000..a2123203 --- /dev/null +++ b/backend/__tests__/utils/networkValidation.dns.test.js @@ -0,0 +1,128 @@ +/** + * DNS-resolving SSRF guard (GHSA SSRF cluster: webhook / S3 / rsync / SMTP / + * IMAP). The literal isPrivateIP check can't see that a public-looking + * hostname resolves to an internal/metadata IP; isHostAllowed resolves the + * name and vets every A/AAAA record. + */ +jest.mock('dns', () => { + const actual = jest.requireActual('dns'); + return { ...actual, promises: { ...actual.promises, lookup: jest.fn() } }; +}); +const dns = require('dns'); +const { + isHostAllowed, + validateExternalUrlAsync, + classifyHost, +} = require('../../src/utils/networkValidation'); + +const lookup = dns.promises.lookup; + +describe('classifyHost', () => { + beforeEach(() => lookup.mockReset()); + + it('distinguishes private, unresolved, ok, and invalid', async () => { + lookup.mockResolvedValue([{ address: '10.0.0.5', family: 4 }]); + expect(await classifyHost('evil.example')).toBe('private'); + + lookup.mockResolvedValue([{ address: '93.184.216.34', family: 4 }]); + expect(await classifyHost('example.com')).toBe('ok'); + + lookup.mockRejectedValue(new Error('EAI_AGAIN')); + expect(await classifyHost('blip.example')).toBe('unresolved'); + + lookup.mockResolvedValue([]); + expect(await classifyHost('empty.example')).toBe('unresolved'); + + expect(await classifyHost('')).toBe('invalid'); + expect(await classifyHost('10.0.0.1')).toBe('private'); // literal, no lookup + }); +}); + +describe('isHostAllowed', () => { + beforeEach(() => lookup.mockReset()); + + it('rejects a public hostname that resolves to a private IP', async () => { + lookup.mockResolvedValue([{ address: '10.0.0.5', family: 4 }]); + expect(await isHostAllowed('evil.example.com')).toBe(false); + }); + + it('rejects when the hostname resolves to the cloud metadata IP', async () => { + lookup.mockResolvedValue([{ address: '169.254.169.254', family: 4 }]); + expect(await isHostAllowed('metadata-rebind.example')).toBe(false); + }); + + it('rejects when ANY resolved address is private (rebinding / mixed records)', async () => { + lookup.mockResolvedValue([ + { address: '93.184.216.34', family: 4 }, + { address: '169.254.169.254', family: 4 }, + ]); + expect(await isHostAllowed('rebind.example')).toBe(false); + }); + + it('allows a hostname that resolves only to public IPs', async () => { + lookup.mockResolvedValue([{ address: '93.184.216.34', family: 4 }]); + expect(await isHostAllowed('example.com')).toBe(true); + }); + + it('fails closed when resolution errors', async () => { + lookup.mockRejectedValue(new Error('ENOTFOUND')); + expect(await isHostAllowed('nxdomain.invalid')).toBe(false); + }); + + it('fails closed on an empty resolution', async () => { + lookup.mockResolvedValue([]); + expect(await isHostAllowed('empty.example')).toBe(false); + }); + + it('rejects literal private IPs and blocked names without resolving', async () => { + expect(await isHostAllowed('127.0.0.1')).toBe(false); + expect(await isHostAllowed('10.0.0.1')).toBe(false); + expect(await isHostAllowed('localhost')).toBe(false); + expect(await isHostAllowed('metadata.google.internal')).toBe(false); + expect(await isHostAllowed('foo.internal')).toBe(false); + expect(lookup).not.toHaveBeenCalled(); + }); + + it('allows a public IP literal without resolving', async () => { + expect(await isHostAllowed('93.184.216.34')).toBe(true); + expect(lookup).not.toHaveBeenCalled(); + }); + + it('rejects empty / non-string input', async () => { + expect(await isHostAllowed('')).toBe(false); + expect(await isHostAllowed(null)).toBe(false); + }); +}); + +describe('validateExternalUrlAsync', () => { + beforeEach(() => lookup.mockReset()); + + it('rejects a URL whose host resolves to a private address', async () => { + lookup.mockResolvedValue([{ address: '10.1.2.3', family: 4 }]); + const r = await validateExternalUrlAsync('https://evil.example/hook'); + expect(r.valid).toBe(false); + }); + + it('accepts a URL whose host resolves public', async () => { + lookup.mockResolvedValue([{ address: '93.184.216.34', family: 4 }]); + expect((await validateExternalUrlAsync('https://example.com/hook')).valid).toBe(true); + }); + + it('rejects a malformed URL', async () => { + expect((await validateExternalUrlAsync('not a url')).valid).toBe(false); + }); + + it('reports reason=unresolved for a transient lookup failure (retryable)', async () => { + lookup.mockRejectedValue(new Error('EAI_AGAIN')); + const r = await validateExternalUrlAsync('https://blip.example/hook'); + expect(r.valid).toBe(false); + expect(r.reason).toBe('unresolved'); + }); + + it('reports reason=private for a resolved-private host (permanent)', async () => { + lookup.mockResolvedValue([{ address: '169.254.169.254', family: 4 }]); + const r = await validateExternalUrlAsync('https://rebind.example/hook'); + expect(r.valid).toBe(false); + expect(r.reason).toBe('private'); + }); +}); diff --git a/backend/src/routes/adminBackup.js b/backend/src/routes/adminBackup.js index c60b2d31..1dedbe4e 100644 --- a/backend/src/routes/adminBackup.js +++ b/backend/src/routes/adminBackup.js @@ -63,14 +63,33 @@ router.put('/config', adminAuth, requirePermission('backup.create'), async (req, } break; case 's3': - if (!updates.backup_s3_endpoint || !updates.backup_s3_bucket || + if (!updates.backup_s3_endpoint || !updates.backup_s3_bucket || !updates.backup_s3_access_key || !updates.backup_s3_secret_key) { return res.status(400).json({ error: 'S3 backup requires endpoint, bucket, and credentials' }); } break; } } - + + // SSRF: validate an S3 endpoint whenever one is supplied — NOT only when + // the payload also flips backup_destination_type to 's3'. The PUT + // persists every backup_* field independently, so with S3 already + // selected a caller could PATCH just backup_s3_endpoint to a + // private-resolving host; the management ops (manifest, bucket/file + // browse, cleanup, test-upload) then connect without going through + // testConnection. Prod-only; dev points at localhost MinIO deliberately. + if (process.env.NODE_ENV === 'production' + && updates.backup_s3_endpoint && updates.backup_s3_endpoint !== '••••••••') { + const rawEndpoint = updates.backup_s3_endpoint; + const withProto = /^https?:\/\//.test(rawEndpoint) ? rawEndpoint : `https://${rawEndpoint}`; + let epHost = null; + try { epHost = new URL(withProto).hostname; } catch { epHost = null; } + const { isHostAllowed } = require('../utils/networkValidation'); + if (!epHost || !(await isHostAllowed(epHost))) { + return res.status(400).json({ error: 'S3 endpoint resolves to a private or internal network address' }); + } + } + // Update settings for (const [key, value] of Object.entries(updates)) { // An unchanged secret round-trips as the GET mask sentinel — keep the @@ -364,9 +383,11 @@ router.post('/test-connection', adminAuth, requirePermission('backup.create'), a break; } - // SSRF protection: block connections to private/internal addresses - const { isPrivateIP } = require('../utils/networkValidation'); - if (isPrivateIP(host)) { + // SSRF protection: resolve the host and block any private/internal + // address. ssh does its own DNS at connect time, so a literal-only + // check let a hostname resolving to an internal IP through (#GHSA-4jh8). + const { isHostAllowed } = require('../utils/networkValidation'); + if (!(await isHostAllowed(host))) { res.json({ success: false, message: 'Host cannot be a private or internal network address' }); break; } diff --git a/backend/src/routes/adminEmail.js b/backend/src/routes/adminEmail.js index 169c6061..f23f1a69 100644 --- a/backend/src/routes/adminEmail.js +++ b/backend/src/routes/adminEmail.js @@ -66,9 +66,11 @@ router.post('/config', [ tls_reject_unauthorized } = req.body; - // Validate SMTP host is not a private/internal address (SSRF protection) - const { isPrivateIP } = require('../utils/networkValidation'); - if (isPrivateIP(smtp_host)) { + // Validate SMTP host is not a private/internal address (SSRF protection). + // Resolves DNS so a public-looking hostname pointing at an internal IP + // is caught, not just literal private addresses (#GHSA-ch64). + const { isHostAllowed } = require('../utils/networkValidation'); + if (!(await isHostAllowed(smtp_host))) { return res.status(400).json({ error: 'SMTP host cannot point to a private or internal network address' }); } @@ -152,8 +154,8 @@ router.post('/incoming-config', [ const errors = validationResult(req); if (!errors.isEmpty()) return res.status(400).json({ errors: errors.array() }); const { imap_host, imap_port, imap_secure, imap_user, imap_pass, imap_folder } = req.body; - const { isPrivateIP } = require('../utils/networkValidation'); - if (isPrivateIP(imap_host)) { + const { isHostAllowed } = require('../utils/networkValidation'); + if (!(await isHostAllowed(imap_host))) { return res.status(400).json({ error: 'IMAP host cannot point to a private or internal network address' }); } const existing = await db('email_configs').first(); @@ -183,8 +185,8 @@ router.post('/incoming-config/folders', adminAuth, requirePermission('email.view try { const { imap_host, imap_port, imap_secure, imap_user, imap_pass } = req.body || {}; if (imap_host) { - const { isPrivateIP } = require('../utils/networkValidation'); - if (isPrivateIP(imap_host)) { + const { isHostAllowed } = require('../utils/networkValidation'); + if (!(await isHostAllowed(imap_host))) { return res.status(400).json({ error: 'IMAP host cannot point to a private or internal network address' }); } } @@ -205,8 +207,8 @@ router.post('/incoming-config/test', adminAuth, requirePermission('email.view'), try { const { imap_host, imap_port, imap_secure, imap_user, imap_pass, imap_folder } = req.body || {}; if (imap_host) { - const { isPrivateIP } = require('../utils/networkValidation'); - if (isPrivateIP(imap_host)) { + const { isHostAllowed } = require('../utils/networkValidation'); + if (!(await isHostAllowed(imap_host))) { return res.status(400).json({ error: 'IMAP host cannot point to a private or internal network address' }); } } @@ -385,11 +387,11 @@ router.post('/accounts', adminAuth, messagingGate, requirePermission('email.edit if (!b.account_key) return res.status(400).json({ error: 'account_key is required' }); // SSRF guard — mirror /config + /incoming-config: neither the IMAP nor the // SMTP host may point at a private/internal address. - const { isPrivateIP } = require('../utils/networkValidation'); - if (b.imap_host && isPrivateIP(b.imap_host)) { + const { isHostAllowed } = require('../utils/networkValidation'); + if (b.imap_host && !(await isHostAllowed(b.imap_host))) { return res.status(400).json({ error: 'IMAP host cannot point to a private or internal network address' }); } - if (b.smtp_host && isPrivateIP(b.smtp_host)) { + if (b.smtp_host && !(await isHostAllowed(b.smtp_host))) { return res.status(400).json({ error: 'SMTP host cannot point to a private or internal network address' }); } const patch = { @@ -434,8 +436,8 @@ router.post('/accounts', adminAuth, messagingGate, requirePermission('email.edit router.post('/accounts/test', adminAuth, messagingGate, requirePermission('email.view'), async (req, res) => { try { const b = req.body || {}; - const { isPrivateIP } = require('../utils/networkValidation'); - if (b.imap_host && isPrivateIP(b.imap_host)) { + const { isHostAllowed } = require('../utils/networkValidation'); + if (b.imap_host && !(await isHostAllowed(b.imap_host))) { return res.status(400).json({ error: 'IMAP host cannot point to a private or internal network address' }); } let pass = b.imap_pass; diff --git a/backend/src/routes/adminWebhooks.js b/backend/src/routes/adminWebhooks.js index fa1d4fec..2ed920a6 100644 --- a/backend/src/routes/adminWebhooks.js +++ b/backend/src/routes/adminWebhooks.js @@ -20,7 +20,7 @@ const { body, query, validationResult } = require('express-validator'); const { db, logActivity } = require('../database/db'); const { adminAuth } = require('../middleware/auth'); const { requirePermission } = require('../middleware/permissions'); -const { validateExternalUrl } = require('../utils/networkValidation'); +const { validateExternalUrlAsync } = require('../utils/networkValidation'); const webhookService = require('../services/webhookService'); const logger = require('../utils/logger'); @@ -78,9 +78,9 @@ router.post( requirePermission('settings.edit'), [ body('name').isString().trim().isLength({ min: 1, max: 100 }), - body('url').isString().isLength({ max: 2048 }).custom((url) => { + body('url').isString().isLength({ max: 2048 }).custom(async (url) => { if (ALLOW_PRIVATE_URLS) return true; - const check = validateExternalUrl(url); + const check = await validateExternalUrlAsync(url); if (!check.valid) throw new Error(check.error); return true; }), @@ -160,9 +160,9 @@ router.put( requirePermission('settings.edit'), [ body('name').optional().isString().trim().isLength({ min: 1, max: 100 }), - body('url').optional().isString().isLength({ max: 2048 }).custom((url) => { + body('url').optional().isString().isLength({ max: 2048 }).custom(async (url) => { if (ALLOW_PRIVATE_URLS) return true; - const check = validateExternalUrl(url); + const check = await validateExternalUrlAsync(url); if (!check.valid) throw new Error(check.error); return true; }), diff --git a/backend/src/services/backupService.js b/backend/src/services/backupService.js index 88db2666..1731a5ba 100644 --- a/backend/src/services/backupService.js +++ b/backend/src/services/backupService.js @@ -845,6 +845,14 @@ function parseRsyncStats(output) { async function performRsyncBackup(config, files) { const { spawnAsync } = require('../utils/safeExec'); + // SSRF: the /test-connection route validates the host, but a scheduled or + // manual /run reaches here directly with the stored host. Resolve-and-vet + // it right before ssh/rsync does its own DNS at connect time, so a host + // that resolves to an internal address can't be reached (GHSA-4jh8). + const { isHostAllowed } = require('../utils/networkValidation'); + if (!(await isHostAllowed(config.backup_rsync_host))) { + throw new Error('rsync host resolves to a private or internal network address'); + } // Anchored excludes for the de-selected What-to-Backup paths; rsync // otherwise transfers the whole storage root regardless of the walker's // file list (which only feeds manifests and file state). diff --git a/backend/src/services/storage/s3Storage.js b/backend/src/services/storage/s3Storage.js index b3fd8766..f7bd0a90 100644 --- a/backend/src/services/storage/s3Storage.js +++ b/backend/src/services/storage/s3Storage.js @@ -131,6 +131,19 @@ class S3StorageAdapter extends stream.EventEmitter { */ async testConnection() { try { + // Resolve-and-vet the custom endpoint before the network round-trip + // (the constructor's literal check can't catch a public-looking + // hostname that resolves to an internal IP). Prod-only, matching the + // constructor gate — dev points at localhost MinIO deliberately. + if (process.env.NODE_ENV === 'production' && this.config.endpoint) { + const { isHostAllowed } = require('../../utils/networkValidation'); + const { hostname } = new URL( + /^https?:\/\//.test(this.config.endpoint) ? this.config.endpoint : `https://${this.config.endpoint}` + ); + if (!(await isHostAllowed(hostname))) { + throw new Error('S3 endpoint resolves to a private or internal network address'); + } + } await this.s3Client.send(new HeadBucketCommand({ Bucket: this.bucket })); logger.info(`Successfully connected to S3 bucket: ${this.bucket}`); return true; diff --git a/backend/src/services/webhookDeliveryWorker.js b/backend/src/services/webhookDeliveryWorker.js index 4a82eee3..a619e05b 100644 --- a/backend/src/services/webhookDeliveryWorker.js +++ b/backend/src/services/webhookDeliveryWorker.js @@ -2,7 +2,7 @@ const axios = require('axios'); const { db } = require('../database/db'); const logger = require('../utils/logger'); const { signPayload, renderTemplate } = require('./webhookService'); -const { validateExternalUrl } = require('../utils/networkValidation'); +const { validateExternalUrlAsync } = require('../utils/networkValidation'); const POLL_INTERVAL_MS = parseInt(process.env.WEBHOOK_DELIVERY_INTERVAL_MS || '5000', 10); const CONCURRENCY = parseInt(process.env.WEBHOOK_DELIVERY_CONCURRENCY || '5', 10); @@ -91,12 +91,24 @@ async function deliverOne(row) { return; } - // Re-validate URL per delivery — DNS-rebinding mitigation. Admin can opt - // out via WEBHOOK_ALLOW_PRIVATE_URLS=true for local-receiver dev runs. + // Re-validate URL per delivery — DNS-rebinding mitigation. Resolves the + // host and vets every A/AAAA record (a public-looking name that now + // resolves to an internal IP is rejected). Admin can opt out via + // WEBHOOK_ALLOW_PRIVATE_URLS=true for local-receiver dev runs. if (!allowPrivateUrls) { - const urlCheck = validateExternalUrl(webhook.url); + const urlCheck = await validateExternalUrlAsync(webhook.url); if (!urlCheck.valid) { - await markFailedFinal(row, `URL rejected: ${urlCheck.error}`); + // A transient lookup failure ('unresolved' — EAI_AGAIN, resolver + // briefly down) must NOT connect: falling through to axios would let + // an attacker SERVFAIL this preflight and answer axios's own lookup + // with a private/metadata IP, defeating the guard. Schedule the + // normal retry/backoff instead — no request is made. A confirmed + // policy rejection (resolves-to-private / malformed) is permanent. + if (urlCheck.reason === 'unresolved') { + await scheduleTransientRetry(row, webhook, 'URL host did not resolve — retrying'); + } else { + await markFailedFinal(row, `URL rejected: ${urlCheck.error}`); + } return; } } @@ -217,6 +229,36 @@ async function markFailedFinal(row, reason) { await db('webhooks').where({ id: row.webhook_id }).update({ last_failure_at: new Date() }); } +// Schedule the normal retry/backoff for a transient failure that must not +// make a network request (e.g. the SSRF preflight lookup failed). Mirrors +// the failure branch of the main delivery path: retry until MAX_ATTEMPTS, +// then give up. No response fields — nothing was sent. +async function scheduleTransientRetry(row, webhook, errorMsg) { + const newAttempt = row.attempt_count + 1; + if (newAttempt >= MAX_ATTEMPTS) { + await db('webhook_deliveries') + .where({ id: row.id }) + .update({ + status: 'failed', + last_error: errorMsg, + attempt_count: newAttempt, + completed_at: new Date(), + next_retry_at: null, + }); + } else { + const backoff = BACKOFF_MS[Math.min(newAttempt - 1, BACKOFF_MS.length - 1)]; + await db('webhook_deliveries') + .where({ id: row.id }) + .update({ + status: 'pending', + last_error: errorMsg, + attempt_count: newAttempt, + next_retry_at: new Date(Date.now() + backoff), + }); + } + await db('webhooks').where({ id: webhook.id }).update({ last_failure_at: new Date() }); +} + function stringifyBody(data) { if (data == null) return null; if (typeof data === 'string') return data; diff --git a/backend/src/utils/networkValidation.js b/backend/src/utils/networkValidation.js index d9b60d0b..8ad21af7 100644 --- a/backend/src/utils/networkValidation.js +++ b/backend/src/utils/networkValidation.js @@ -1,5 +1,6 @@ const { URL } = require('url'); const net = require('net'); +const dns = require('dns').promises; /** * Check if a hostname or IP resolves to a private/internal network address. @@ -162,6 +163,11 @@ function isPrivateIPv6(ip) { /** * Validate a URL string, rejecting private/internal targets. + * + * NOTE: literal-only. For a hostname (not an IP), this checks the string but + * NOT what it resolves to — `evil.example` with an A record of 10.0.0.5 + * passes. Prefer isHostAllowed / validateExternalUrlAsync at any call site + * that then actually connects; kept for synchronous callers and fast checks. * @param {string} urlString - URL to validate * @returns {{ valid: boolean, error?: string }} */ @@ -177,4 +183,65 @@ function validateExternalUrl(urlString) { } } -module.exports = { isPrivateIP, validateExternalUrl }; +/** + * Resolve a hostname and reject if it (or ANY of its A/AAAA records) points + * at a private/internal address. Closes the SSRF hole where a public-looking + * hostname resolves to an internal IP or the cloud metadata endpoint — the + * literal isPrivateIP check alone can't see that. Fails closed on resolution + * failure. IP literals are decided by isPrivateIP without a lookup. + * + * Residual: a determined attacker who controls DNS can still rebind between + * this check and the client's own resolution (TOCTOU). Fully closing that + * needs pinning the connection to the vetted IP, which the underlying + * clients (nodemailer/imap/ssh/aws-sdk) don't cleanly support; these actions + * are admin-only, so resolve-and-vet is the proportionate mitigation. + * + * @param {string} hostname + * @returns {Promise} true when safe to connect + */ +async function classifyHost(hostname) { + if (!hostname || typeof hostname !== 'string') return 'invalid'; + // Literal check first: IP literals, blocked names, .internal/.local/.localhost. + if (isPrivateIP(hostname)) return 'private'; + // An IP literal is fully decided above — no name to resolve. + const bare = hostname.replace(/^\[|\]$/g, ''); + if (net.isIP(bare)) return 'ok'; + let addresses; + try { + addresses = await dns.lookup(hostname, { all: true }); + } catch { + return 'unresolved'; // transient/NXDOMAIN — caller decides retry vs reject + } + if (!addresses.length) return 'unresolved'; + return addresses.every((a) => !isPrivateIP(a.address)) ? 'ok' : 'private'; +} + +async function isHostAllowed(hostname) { + // Fail-closed boolean for save/test call sites: anything not clearly 'ok' + // (including a transient lookup failure) is rejected. + return (await classifyHost(hostname)) === 'ok'; +} + +/** + * Async, DNS-resolving counterpart to validateExternalUrl. Returns a `reason` + * so callers with retry semantics (e.g. the webhook worker) can distinguish a + * policy rejection ('private'/'invalid') from a transient lookup failure + * ('unresolved') that should be retried rather than permanently failed. + * @param {string} urlString + * @returns {Promise<{ valid: boolean, error?: string, reason: string }>} + */ +async function validateExternalUrlAsync(urlString) { + let parsed; + try { + parsed = new URL(urlString); + } catch { + return { valid: false, error: 'Invalid URL format', reason: 'invalid' }; + } + const reason = await classifyHost(parsed.hostname); + if (reason !== 'ok') { + return { valid: false, error: 'URL points to a private or internal network address', reason }; + } + return { valid: true, reason: 'ok' }; +} + +module.exports = { isPrivateIP, validateExternalUrl, isHostAllowed, validateExternalUrlAsync, classifyHost };