fix(connection): judge an entry's explicit port from the parsed URL, not the raw string
WHATWG trimming strips stray whitespace before parsing, so 'host:port ' passed the load assert while the raw-string port regex read it as port-less — broadening an exact-port grant to every port on that hostname. The explicit- port judgment now reads URL parses under both special schemes (:80/:443 stay explicit), and the load assert refuses whitespace outright.
This commit is contained in:
@@ -45,16 +45,29 @@ function parseAuthority(authority: string): URL | undefined {
|
|||||||
* `host:port`) and nothing else. WHATWG parsing would quietly read a hostname
|
* `host:port`) and nothing else. WHATWG parsing would quietly read a hostname
|
||||||
* out of `harness.internal/path` or `user@harness.internal` — a typo must fail
|
* out of `harness.internal/path` or `user@harness.internal` — a typo must fail
|
||||||
* the load loudly instead of authorizing its hostname or being ignored until
|
* the load loudly instead of authorizing its hostname or being ignored until
|
||||||
* requests 403. The delimiter test refuses every URL part beyond the authority
|
* requests 403. The character test refuses every URL part beyond the authority
|
||||||
* (path, backslash path, query, fragment, userinfo); IPv6 brackets use none of
|
* (path, backslash path, query, fragment, userinfo) and all whitespace, which
|
||||||
|
* WHATWG trimming would otherwise strip silently; IPv6 brackets use none of
|
||||||
* them.
|
* them.
|
||||||
* @param entry - the configured value, verbatim.
|
* @param entry - the configured value, verbatim.
|
||||||
*/
|
*/
|
||||||
export function assertTrustedAuthority(entry: string): void {
|
export function assertTrustedAuthority(entry: string): void {
|
||||||
if (parseAuthority(entry) !== undefined && !/[/\\?#@]/.test(entry)) return
|
if (parseAuthority(entry) !== undefined && !/[/\\?#@\s]/.test(entry)) return
|
||||||
throw new Error(`client-connection: trustedHosts entry ${JSON.stringify(entry)} is not a bare host[:port] authority`)
|
throw new Error(`client-connection: trustedHosts entry ${JSON.stringify(entry)} is not a bare host[:port] authority`)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Whether the parsed authority carries an explicit port: judged from URL
|
||||||
|
* parses under both special schemes (their default ports differ, so `:80` and
|
||||||
|
* `:443` still count as explicit), never from the raw string, where WHATWG
|
||||||
|
* trimming of stray whitespace would misread `host:port ` as port-less and
|
||||||
|
* broaden an exact-port grant to every port.
|
||||||
|
*/
|
||||||
|
function hasExplicitPort(entry: string, entryUrl: URL): boolean {
|
||||||
|
// An authority that parsed under http cannot fail under https.
|
||||||
|
return entryUrl.port !== '' || new URL(`https://${entry}`).port !== ''
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Whether the request authority matches a `trustedHosts` entry. An entry with
|
* Whether the request authority matches a `trustedHosts` entry. An entry with
|
||||||
* an explicit port matches that exact authority; a port-less entry matches the
|
* an explicit port matches that exact authority; a port-less entry matches the
|
||||||
@@ -66,7 +79,7 @@ function isTrustedAuthority(hostUrl: URL, trustedHosts: readonly string[]): bool
|
|||||||
return trustedHosts.some((entry) => {
|
return trustedHosts.some((entry) => {
|
||||||
const entryUrl = parseAuthority(entry)
|
const entryUrl = parseAuthority(entry)
|
||||||
if (entryUrl === undefined) return false
|
if (entryUrl === undefined) return false
|
||||||
return /:\d+$/.test(entry)
|
return hasExplicitPort(entry, entryUrl)
|
||||||
? entryUrl.host === hostUrl.host
|
? entryUrl.host === hostUrl.host
|
||||||
: entryUrl.hostname === hostUrl.hostname
|
: entryUrl.hostname === hostUrl.hostname
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -75,6 +75,18 @@ describe('isTrustedApiRequest', () => {
|
|||||||
for (const entry of ['harness.internal/path', 'harness.internal/', 'user@harness.internal', 'harness.internal?x', 'harness.internal#f', 'harness.internal\\path', 'bad entry', '']) {
|
for (const entry of ['harness.internal/path', 'harness.internal/', 'user@harness.internal', 'harness.internal?x', 'harness.internal#f', 'harness.internal\\path', 'bad entry', '']) {
|
||||||
expect(() => { assertTrustedAuthority(entry) }).toThrow(/not a bare host\[:port\] authority/)
|
expect(() => { assertTrustedAuthority(entry) }).toThrow(/not a bare host\[:port\] authority/)
|
||||||
}
|
}
|
||||||
|
// WHATWG trimming would silently strip these; the entry must fail instead.
|
||||||
|
for (const entry of ['harness.internal:3080 ', ' harness.internal', 'harness.internal:30\t80']) {
|
||||||
|
expect(() => { assertTrustedAuthority(entry) }).toThrow(/not a bare host\[:port\] authority/)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
it('never lets stray whitespace broaden an exact-port entry to every port', () => {
|
||||||
|
// Defense in depth below the load-time assert: the explicit-port judgment
|
||||||
|
// reads the parsed URL, so a trimmed `host:port ` entry stays exact.
|
||||||
|
const trusted = ['harness.internal:3080 ']
|
||||||
|
expect(isTrustedApiRequest(request({ host: 'harness.internal:9999', origin: 'http://harness.internal:9999' }), trusted)).toBe(false)
|
||||||
|
expect(isTrustedApiRequest(request({ host: 'harness.internal:3080', origin: 'http://harness.internal:3080' }), trusted)).toBe(true)
|
||||||
})
|
})
|
||||||
|
|
||||||
it('refuses malformed or untrusted authorities on browser requests', () => {
|
it('refuses malformed or untrusted authorities on browser requests', () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user