Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/node-adapter-request-target-host-confusion.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'mppx': patch
---

Fixed host confusion in the Node adapter (`Request.fromNodeListener`/`toNodeListener`). Protocol-relative (`//evil.com/x`), triple-slash (`///evil.com/x`), backslash (`/\evil.com/x`), and embedded-authority (`//a//evil.com/x`) request targets could previously override the request host derived from the `Host` header, which in turn poisoned the auto-detected challenge `realm`. The adapter now copies only the parsed path and query onto a trusted origin, so the request target's authority can never influence the resulting URL host.
74 changes: 74 additions & 0 deletions src/server/Mppx.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3983,6 +3983,80 @@ describe('realm auto-detection', () => {
}
})

// A raw request line can carry a protocol-relative or embedded-authority
// target that WHATWG URL would resolve into a foreign host, which would then
// be auto-detected as the challenge realm. The realm must stay bound to the
// Host header.
test.each([
{ name: 'protocol-relative', path: '//evil.com/resource' },
{ name: 'embedded authority', path: '//first.example//evil.com/resource' },
])('ignores $name request targets when deriving realm in node listeners', async ({ path }) => {
const handler = Mppx.create({ methods: [mockMethod], secretKey })
const server = await Http.createServer(async (req, res) => {
const result = await Mppx.toNodeListener(
handler.charge({
amount: '100',
currency: '0x0000000000000000000000000000000000000001',
recipient: '0x0000000000000000000000000000000000000002',
}),
)(req, res)

if (result.status !== 402) res.end('OK')
})

try {
const rawResponse = await new Promise<{
body: string
headers: http.IncomingHttpHeaders
statusCode: number
}>((resolve, reject) => {
const request = http.request(
{
host: '127.0.0.1',
port: server.port,
method: 'GET',
path,
headers: { Host: `localhost:${server.port}` },
},
(response) => {
const chunks: Buffer[] = []
response.on('data', (chunk) => chunks.push(Buffer.from(chunk)))
response.on('end', () => {
resolve({
body: Buffer.concat(chunks).toString('utf8'),
headers: response.headers,
statusCode: response.statusCode ?? 0,
})
})
},
)

request.on('error', reject)
request.end()
})

const headers = new Headers()
for (const [name, value] of Object.entries(rawResponse.headers)) {
if (Array.isArray(value)) {
for (const item of value) headers.append(name, item)
} else if (value !== undefined) {
headers.append(name, value)
}
}

const challenge = Challenge.fromResponse(
new Response(rawResponse.body, {
status: rawResponse.statusCode,
headers,
}),
)

expect(challenge.realm).toBe('localhost')
} finally {
server.close()
}
})

test('credential verifies across different casing of same host', async () => {
const handler = Mppx.create({ methods: [mockMethod], secretKey })

Expand Down
81 changes: 81 additions & 0 deletions src/server/Request.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { EventEmitter } from 'node:events'
import { createServer } from 'node:http'
import type { IncomingMessage, ServerResponse } from 'node:http'
import type { AddressInfo } from 'node:net'
import { connect } from 'node:net'

import { Request } from 'mppx/server'
import { describe, expect, test } from 'vp/test'
Expand Down Expand Up @@ -73,6 +76,32 @@ describe('fromNodeListener', () => {
expect(request.url).toBe('http://example.com/api/resource?q=1')
})

// The request target's authority must never override the trusted host
// (options.host > Host > :authority > localhost). A raw request line can
// carry protocol-relative, triple-slash, backslash, or embedded-authority
// targets that WHATWG URL would otherwise resolve into a foreign host.
test.each([
['protocol-relative', '//evil.com/path?q=1', 'http://example.com/path?q=1'],
['triple-slash', '///evil.com/x', 'http://example.com/x'],
['backslash', '/\\evil.com/x', 'http://example.com/x'],
['userinfo authority', '//user:pass@evil.com/path', 'http://example.com/path'],
['ipv6 authority', '//[2001:db8::1]:8443/path', 'http://example.com/path'],
['embedded authority', '//first.example//evil.com/path', 'http://example.com//evil.com/path'],
[
'absolute embedded authority',
'http://first.example//evil.com/p',
'http://example.com//evil.com/p',
],
['encoded slashes stay path', '/%2F%2Fevil.com/p', 'http://example.com/%2F%2Fevil.com/p'],
])('binds host to Host header for %s request targets', (_name, url, expected) => {
const [req, res] = createMockRequest({ url, rawHeaders: ['Host', 'example.com'] })

const request = Request.fromNodeListener(req, res)

expect(request.url).toBe(expected)
expect(new URL(request.url).host).toBe('example.com')
})

test('uses explicit protocol and host overrides', () => {
const [req, res] = createMockRequest({
url: '/api/resource',
Expand Down Expand Up @@ -172,3 +201,55 @@ describe('fromNodeListener', () => {
expect(await request.text()).toBe('hello')
})
})

// Conformance harness: a normal HTTP client cannot emit these request targets,
// so they are driven through Node's real HTTP parser over a raw socket to prove
// the adapter neutralizes host confusion end-to-end (not just via mocks).
describe('toNodeListener (raw request target)', () => {
async function captureRequestUrl(rawRequestLine: string): Promise<string> {
let resolveUrl!: (url: string) => void
const observed = new Promise<string>((resolve) => {
resolveUrl = resolve
})

const server = createServer(
Request.toNodeListener(async (request) => {
resolveUrl(request.url)
return new Response('ok')
}),
)

try {
await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', () => resolve()))
const { port } = server.address() as AddressInfo
const socket = connect(port, '127.0.0.1')
await new Promise<void>((resolve, reject) => {
socket.once('connect', () => resolve())
socket.once('error', reject)
})
socket.write(rawRequestLine)
const url = await observed
socket.destroy()
return url
} finally {
await new Promise<void>((resolve) => server.close(() => resolve()))
}
}

test('binds host to Host header for a protocol-relative raw request target', async () => {
const url = await captureRequestUrl(
'GET //evil.com/protected?q=1 HTTP/1.1\r\nHost: example.com\r\nConnection: close\r\n\r\n',
)

expect(url).toBe('http://example.com/protected?q=1')
expect(new URL(url).host).toBe('example.com')
})

test('binds host to Host header for an embedded-authority raw request target', async () => {
const url = await captureRequestUrl(
'GET //first.example//evil.com/protected HTTP/1.1\r\nHost: example.com\r\nConnection: close\r\n\r\n',
)

expect(new URL(url).host).toBe('example.com')
})
})
32 changes: 23 additions & 9 deletions src/server/Request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export function fromNodeListener(
headers.get('Host') ??
(req.headers as Record<string, string>)[':authority'] ??
'localhost'
const url = new URL(normalizeRequestTarget(req.url), `${protocol}//${host}`)
const url = createRequestUrl(req.url, `${protocol}//${host}`)

const init: RequestInit & { duplex?: string } = {
method,
Expand Down Expand Up @@ -116,17 +116,31 @@ function hasBody(headers: Headers): boolean {
return (contentLength !== null && contentLength !== '0') || headers.has('transfer-encoding')
}

function normalizeRequestTarget(url: string | undefined): string {
if (!url) return '/'
/**
* Builds the request `URL` from a request target and a trusted origin.
*
* Only the parsed `pathname`/`search` are copied onto the trusted origin, so
* the target's authority can never override the host (protocol-relative,
* `///`, backslash, absolute-form, or embedded-authority targets). Components
* are copied onto a `URL` object rather than concatenated and re-parsed, since
* a normalized path can itself begin with `//` and be read as an authority.
*/
function createRequestUrl(target: string | undefined, origin: string): URL {
const url = new URL(origin)
if (!target) return url

let parsed: URL
try {
const absoluteUrl = new URL(url)
// Absolute-form request targets can carry a different origin than the socket host.
// Keep only path+query so realm detection stays bound to Host/:authority.
if (absoluteUrl.protocol === 'http:' || absoluteUrl.protocol === 'https:')
return `${absoluteUrl.pathname}${absoluteUrl.search}`
} catch {}
parsed = new URL(target, 'http://mppx.invalid')
} catch {
throw new TypeError('Invalid request target')
}
if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:')
throw new TypeError('Unsupported request target protocol')

url.pathname = parsed.pathname
url.search = parsed.search
url.hash = ''
return url
}

Expand Down
Loading