harden(d28): SSRF guard + no credential-URL leak on SMS gateway fetch

Security review of 58c9799: (1) block loopback/link-local/private targets for the
admin-set sms.balance_api_url before sending client credentials (assertSafeGatewayUrl
— rejects 127/10/172.16-31/192.168/169.254/localhost/::1/.internal), so the setting
can't be pointed at cloud metadata or internal services; (2) redirect:'manual' so a
3xx can't carry credentials to another host; (3) error messages never interpolate the
credential-bearing URL. http:// transport kept — it is the vendor panel's only
interface and the operator supplied it. Tests cover the block list.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
feat/client-detail-redesign
Thomas Joise 3 days ago
parent 58c97994bd
commit f7e9443a56

@ -32,6 +32,29 @@ export function parseBalanceResponse(raw: string): BalanceResult {
return { ok: false, balance: null, message: `Could not read a balance from: ${text.slice(0, 80)}` }
}
/**
* SSRF guard: the balance URL is an admin-set setting and we send client credentials to
* it, so refuse loopback / link-local / private targets (e.g. cloud metadata 169.254.169.254,
* localhost, RFC-1918). A literal private IP or an internal-looking name throws; public hosts
* pass. Throws a message that NEVER contains the credential-bearing URL. Exported for tests.
*/
export function assertSafeGatewayUrl(base: string): void {
let u: URL
try { u = new URL(base) } catch { throw new Error('Invalid balance API URL') }
if (u.protocol !== 'http:' && u.protocol !== 'https:') throw new Error('Balance API URL must be http or https')
const host = u.hostname.toLowerCase().replace(/^\[|\]$/g, '')
if (host === 'localhost' || host === '::1' || host === '::' || host.endsWith('.local') || host.endsWith('.internal')) {
throw new Error('Balance API host not allowed (internal)')
}
const m = host.match(/^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/)
if (m !== null) {
const a = Number(m[1]!), b = Number(m[2]!)
if (a === 0 || a === 127 || a === 10 || (a === 169 && b === 254) || (a === 192 && b === 168) || (a === 172 && b >= 16 && b <= 31)) {
throw new Error('Balance API host not allowed (private/loopback)')
}
}
}
/** Build the request URL, substituting the account credentials into the configured base. */
export function balanceUrl(base: string, uname: string, pass: string): string {
const u = new URL(base)
@ -41,20 +64,28 @@ export function balanceUrl(base: string, uname: string, pass: string): string {
}
/**
* Call the gateway for one account's balance. Network + parse; a timeout or non-200
* becomes a failed BalanceResult (never throws) so a bad account can't abort a batch.
* Call the gateway for one account's balance. Network + parse; a timeout, redirect, or
* non-200 becomes a failed BalanceResult (never throws) so a bad account can't abort a
* batch. Redirects are NOT followed and the credential-bearing URL is never put into any
* returned message (it would leak the account password to logs/callers).
*/
export async function fetchSmsBalance(base: string, uname: string, pass: string): Promise<BalanceResult> {
let url: string
try { url = balanceUrl(base, uname, pass) } catch { return { ok: false, balance: null, message: 'Invalid balance API URL' } }
try { assertSafeGatewayUrl(base); url = balanceUrl(base, uname, pass) }
catch (e) { return { ok: false, balance: null, message: e instanceof Error ? e.message : 'Invalid balance API URL' } }
try {
const ctrl = new AbortController()
const t = setTimeout(() => ctrl.abort(), 12_000)
const resp = await fetch(url, { signal: ctrl.signal }).finally(() => clearTimeout(t))
// redirect:'manual' — a 3xx must not carry the credentials off to another host.
const resp = await fetch(url, { signal: ctrl.signal, redirect: 'manual' }).finally(() => clearTimeout(t))
if (resp.type === 'opaqueredirect' || (resp.status >= 300 && resp.status < 400)) {
return { ok: false, balance: null, message: 'Gateway redirected — not followed (credential safety)' }
}
if (!resp.ok) return { ok: false, balance: null, message: `Gateway HTTP ${resp.status}` }
return parseBalanceResponse(await resp.text())
} catch (err) {
const msg = err instanceof Error ? err.message : String(err)
return { ok: false, balance: null, message: `Gateway unreachable: ${msg}` }
// Never interpolate the URL/error verbatim — it can carry the credential query string.
const aborted = err instanceof Error && err.name === 'AbortError'
return { ok: false, balance: null, message: aborted ? 'Gateway timed out' : 'Gateway unreachable' }
}
}

@ -1,6 +1,6 @@
// apps/hq/test/sms-gateway.test.ts — D28: parse the SMS gateway's balance response.
import { describe, it, expect } from 'vitest'
import { parseBalanceResponse, balanceUrl } from '../src/sms-gateway'
import { parseBalanceResponse, balanceUrl, assertSafeGatewayUrl } from '../src/sms-gateway'
describe('parseBalanceResponse (D28)', () => {
it('reads the balance from a success line (code|STATUS|balance)', () => {
@ -34,3 +34,19 @@ describe('balanceUrl (D28)', () => {
expect(u).toContain('pass=p%40ss+w%2Fspecial') // URL-encoded
})
})
describe('assertSafeGatewayUrl (D28 SSRF guard)', () => {
it('allows the vendor panel and other public hosts', () => {
expect(() => assertSafeGatewayUrl('http://164.52.195.161/API/BalAlert.aspx')).not.toThrow()
expect(() => assertSafeGatewayUrl('https://sms.vendor.example/bal')).not.toThrow()
})
it('blocks loopback, link-local metadata, and private ranges', () => {
for (const bad of [
'http://127.0.0.1/x', 'http://localhost/x', 'http://169.254.169.254/latest/meta-data',
'http://10.0.0.5/x', 'http://192.168.1.5/x', 'http://172.16.0.1/x', 'http://[::1]/x',
'http://box.internal/x', 'ftp://164.52.195.161/x',
]) {
expect(() => assertSafeGatewayUrl(bad), bad).toThrow()
}
})
})

Loading…
Cancel
Save