Skip to content

Commit 500b9d5

Browse files
committed
fixes an issue with x-forwarded headers and checkOrigin
1 parent a509941 commit 500b9d5

File tree

4 files changed

+67
-45
lines changed

4 files changed

+67
-45
lines changed

.changeset/many-flies-shine.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'astro': patch
3+
---
4+
5+
Enable checkOrigin when astro is behind a reverse proxy

packages/astro/src/core/app/node.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,17 @@ interface NodeRequest extends IncomingMessage {
1919
body?: unknown;
2020
}
2121

22+
function findRequestPort(req: NodeRequest): string | undefined {
23+
const hostheader = req.headers['host'];
24+
if (hostheader) {
25+
const hosturl = new URL(`https://${hostheader}`)
26+
if (hosturl.port) {
27+
return hosturl.port
28+
}
29+
}
30+
return undefined
31+
}
32+
2233
/**
2334
* Converts a NodeJS IncomingMessage into a web standard Request.
2435
* ```js
@@ -76,7 +87,7 @@ export function createRequest(
7687
allowedDomains,
7788
);
7889
const hostname = validated.host ?? validatedHostname ?? 'localhost';
79-
const port = validated.port;
90+
const port = validated.port ?? findRequestPort(req)
8091

8192
let url: URL;
8293
try {

packages/astro/src/core/app/validate-headers.ts

Lines changed: 37 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { matchPattern, type RemotePattern } from '@astrojs/internal-helpers/remote';
1+
import { matchPattern, matchHostname, type RemotePattern } from '@astrojs/internal-helpers/remote';
22

33
/**
44
* Sanitize a hostname by rejecting any with path separators.
@@ -85,57 +85,52 @@ export function validateForwardedHeaders(
8585
): { protocol?: string; host?: string; port?: string } {
8686
const result: { protocol?: string; host?: string; port?: string } = {};
8787

88-
// Validate protocol
89-
if (forwardedProtocol) {
90-
if (allowedDomains && allowedDomains.length > 0) {
91-
const hasProtocolPatterns = allowedDomains.some((pattern) => pattern.protocol !== undefined);
92-
if (hasProtocolPatterns) {
93-
// Validate against allowedDomains patterns
94-
try {
95-
const testUrl = new URL(`${forwardedProtocol}://example.com`);
96-
const isAllowed = allowedDomains.some((pattern) => matchPattern(testUrl, pattern));
97-
if (isAllowed) {
98-
result.protocol = forwardedProtocol;
99-
}
100-
} catch {
101-
// Invalid protocol, omit from result
102-
}
103-
} else if (/^https?$/.test(forwardedProtocol)) {
104-
// allowedDomains exist but no protocol patterns, allow http/https
105-
result.protocol = forwardedProtocol;
106-
}
107-
} else if (/^https?$/.test(forwardedProtocol)) {
108-
// No allowedDomains, only allow http/https
109-
result.protocol = forwardedProtocol;
110-
}
88+
// Require allowed domains to validate any forwarded headers - prevents trusting unvalidated headers
89+
if (!allowedDomains || allowedDomains.length === 0) {
90+
return result
11191
}
11292

113-
// Validate port first
114-
if (forwardedPort && allowedDomains && allowedDomains.length > 0) {
115-
const hasPortPatterns = allowedDomains.some((pattern) => pattern.port !== undefined);
116-
if (hasPortPatterns) {
117-
// Validate against allowedDomains patterns
118-
const isAllowed = allowedDomains.some((pattern) => pattern.port === forwardedPort);
119-
if (isAllowed) {
120-
result.port = forwardedPort;
121-
}
122-
}
123-
// If no port patterns, reject the header (strict security default)
124-
}
93+
let allowedDomain: RemotePattern = {}
12594

12695
// Validate host (extract port from hostname for validation)
12796
// Reject empty strings and sanitize to prevent path injection
128-
if (forwardedHost && forwardedHost.length > 0 && allowedDomains && allowedDomains.length > 0) {
129-
const protoForValidation = result.protocol || 'https';
97+
if (forwardedHost && forwardedHost.length > 0) {
13098
const sanitized = sanitizeHost(forwardedHost);
13199
if (sanitized) {
132-
const { hostname, port: portFromHost } = parseHost(sanitized);
133-
const portForValidation = result.port || portFromHost;
134-
if (matchesAllowedDomains(hostname, protoForValidation, portForValidation, allowedDomains)) {
135-
result.host = sanitized;
100+
const { hostname } = parseHost(sanitized);
101+
if (hostname) {
102+
const testUrl = new URL(`https://${hostname}/`);
103+
// do not match anything other than hostname here.
104+
const found = allowedDomains.find((pattern) => matchHostname(testUrl, pattern.hostname, true));
105+
if (found) {
106+
result.host = sanitized;
107+
allowedDomain = found
108+
}
136109
}
137110
}
138111
}
139112

113+
// If host is not valid, we cannot trust protocol or port from forwarded headers
114+
if (!result.host || !allowedDomain.hostname) {
115+
return result;
116+
}
117+
118+
// Validate port
119+
if (forwardedPort && allowedDomain.port && allowedDomain.port === forwardedPort) {
120+
result.port = forwardedPort;
121+
}
122+
123+
// Validate protocol
124+
if (forwardedProtocol) {
125+
if (allowedDomain.protocol) {
126+
if (allowedDomain.protocol === forwardedProtocol) {
127+
result.protocol = forwardedProtocol;
128+
}
129+
} else if (/^https?$/.test(forwardedProtocol)) {
130+
// allowdDomains does not contain protocol, allow protocol
131+
result.protocol = forwardedProtocol;
132+
}
133+
}
134+
140135
return result;
141136
}

packages/astro/test/units/app/node.test.js

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ describe('node', () => {
7777
...mockNodeRequest,
7878
headers: {
7979
'x-forwarded-host': 'www2.example.com,www3.example.com',
80+
'x-forwarded-proto': 'https',
8081
},
8182
},
8283
{ allowedDomains: [{ hostname: '**.example.com' }] },
@@ -324,6 +325,7 @@ describe('node', () => {
324325
...mockNodeRequest,
325326
headers: {
326327
host: 'example.com',
328+
'x-forwarded-host': 'example.com',
327329
'x-forwarded-proto': 'http',
328330
'x-forwarded-port': '80',
329331
},
@@ -339,6 +341,7 @@ describe('node', () => {
339341
...mockNodeRequest,
340342
headers: {
341343
host: 'example.com',
344+
'x-forwarded-host': 'example.com',
342345
'x-forwarded-proto': 'http,https',
343346
'x-forwarded-port': '80,443',
344347
},
@@ -354,6 +357,7 @@ describe('node', () => {
354357
...mockNodeRequest,
355358
headers: {
356359
host: 'example.com',
360+
'x-forwarded-host': 'example.com',
357361
},
358362
},
359363
{ allowedDomains: [{ hostname: 'example.com' }] },
@@ -367,6 +371,7 @@ describe('node', () => {
367371
...mockNodeRequest,
368372
headers: {
369373
host: 'example.com',
374+
'x-forwarded-host': 'example.com',
370375
'x-forwarded-proto': 'https://www.malicious-url.com/?tank=',
371376
},
372377
},
@@ -381,6 +386,7 @@ describe('node', () => {
381386
...mockNodeRequest,
382387
headers: {
383388
host: 'example.com',
389+
'x-forwarded-host': 'example.com',
384390
'x-forwarded-proto': 'x:admin?',
385391
},
386392
},
@@ -395,6 +401,7 @@ describe('node', () => {
395401
...mockNodeRequest,
396402
headers: {
397403
host: 'example.com',
404+
'x-forwarded-host': 'example.com',
398405
'x-forwarded-proto': 'https://localhost/vulnerable?',
399406
},
400407
},
@@ -409,6 +416,7 @@ describe('node', () => {
409416
...mockNodeRequest,
410417
headers: {
411418
host: 'example.com',
419+
'x-forwarded-host': 'example.com',
412420
'x-forwarded-proto': 'javascript:alert(document.cookie)//',
413421
},
414422
},
@@ -423,6 +431,7 @@ describe('node', () => {
423431
...mockNodeRequest,
424432
headers: {
425433
host: 'example.com',
434+
'x-forwarded-host': 'example.com',
426435
'x-forwarded-proto': '',
427436
},
428437
},
@@ -439,12 +448,12 @@ describe('node', () => {
439448
...mockNodeRequest,
440449
headers: {
441450
host: 'example.com',
451+
'x-forwarded-host': 'example.com',
442452
'x-forwarded-port': '8443',
443453
},
444454
},
445455
{
446456
allowedDomains: [
447-
{ hostname: 'example.com' },
448457
{ hostname: 'example.com', port: '8443' },
449458
],
450459
},
@@ -458,12 +467,12 @@ describe('node', () => {
458467
...mockNodeRequest,
459468
headers: {
460469
host: 'example.com',
470+
'x-forwarded-host': 'example.com',
461471
'x-forwarded-port': '8443,3000',
462472
},
463473
},
464474
{
465475
allowedDomains: [
466-
{ hostname: 'example.com' },
467476
{ hostname: 'example.com', port: '8443' },
468477
],
469478
},
@@ -477,6 +486,7 @@ describe('node', () => {
477486
...mockNodeRequest,
478487
headers: {
479488
host: 'example.com',
489+
'x-forwarded-host': 'example.com',
480490
'x-forwarded-port': '8443',
481491
},
482492
},
@@ -491,6 +501,7 @@ describe('node', () => {
491501
...mockNodeRequest,
492502
headers: {
493503
host: 'example.com:3000',
504+
'x-forwarded-host': 'example.com',
494505
'x-forwarded-port': '443',
495506
},
496507
},

0 commit comments

Comments
 (0)