Skip to content

Commit 42a1bb9

Browse files
qzioqzio
committed
fixes an issue with x-forwarded headers and checkOrigin
1 parent 55c568c commit 42a1bb9

File tree

5 files changed

+79
-50
lines changed

5 files changed

+79
-50
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, proto: string): string | undefined {
23+
const hostheader = req.headers['host'];
24+
if (hostheader) {
25+
const hosturl = new URL(`${proto}://${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 ?? req.socket?.localPort?.toString() ?? findRequestPort(req, protocol);
8091

8192
let url: URL;
8293
try {

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

Lines changed: 42 additions & 41 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,55 +85,56 @@ 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;
88+
// Require allowed domains to validate any forwarded headers - prevents trusting unvalidated headers
89+
if (!allowedDomains || allowedDomains.length === 0) {
90+
return result
91+
}
92+
93+
let allowedDomain: RemotePattern = {}
94+
95+
// Validate host (extract port from hostname for validation)
96+
// Reject empty strings and sanitize to prevent path injection
97+
if (forwardedHost && forwardedHost.length > 0) {
98+
const sanitized = sanitizeHost(forwardedHost);
99+
if (sanitized) {
100+
const { hostname, port } = 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+
// also check for ports in the forwarded header, if there is a mismatch, return
107+
if (port) {
108+
if (found.port && found.port !== port) {
109+
return result;
110+
}
99111
}
100-
} catch {
101-
// Invalid protocol, omit from result
112+
result.host = sanitized;
113+
allowedDomain = found
102114
}
103-
} else if (/^https?$/.test(forwardedProtocol)) {
104-
// allowedDomains exist but no protocol patterns, allow http/https
105-
result.protocol = forwardedProtocol;
106115
}
107-
} else if (/^https?$/.test(forwardedProtocol)) {
108-
// No allowedDomains, only allow http/https
109-
result.protocol = forwardedProtocol;
110116
}
111117
}
112118

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)
119+
// If host is not valid, we cannot trust protocol or port from forwarded headers
120+
if (!result.host || !allowedDomain.hostname) {
121+
return result;
124122
}
125123

126-
// Validate host (extract port from hostname for validation)
127-
// 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';
130-
const sanitized = sanitizeHost(forwardedHost);
131-
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;
124+
// Validate port
125+
if (forwardedPort && allowedDomain.port && allowedDomain.port === forwardedPort) {
126+
result.port = forwardedPort;
127+
}
128+
129+
// Validate protocol
130+
if (forwardedProtocol) {
131+
if (allowedDomain.protocol) {
132+
if (allowedDomain.protocol === forwardedProtocol) {
133+
result.protocol = forwardedProtocol;
136134
}
135+
} else if (/^https?$/.test(forwardedProtocol)) {
136+
// allowdDomains does not contain protocol, allow protocol
137+
result.protocol = forwardedProtocol;
137138
}
138139
}
139140

‎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
},

‎packages/integrations/node/test/url.test.js‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ describe('URL', () => {
4848
it('return http when the X-Forwarded-Proto header is set to http', async () => {
4949
const { handler } = await import('./fixtures/url/dist/server/entry.mjs');
5050
const { req, res, text } = createRequestAndResponse({
51-
headers: { 'X-Forwarded-Proto': 'http' },
51+
headers: { 'X-Forwarded-Proto': 'http', 'X-Forwarded-Host': 'legitimate.example.com' },
5252
url: '/',
5353
});
5454

@@ -62,7 +62,7 @@ describe('URL', () => {
6262
it('return https when the X-Forwarded-Proto header is set to https', async () => {
6363
const { handler } = await import('./fixtures/url/dist/server/entry.mjs');
6464
const { req, res, text } = createRequestAndResponse({
65-
headers: { 'X-Forwarded-Proto': 'https' },
65+
headers: { 'X-Forwarded-Proto': 'https', 'X-Forwarded-Host': 'legitimate.example.com' },
6666
url: '/',
6767
});
6868

@@ -71,6 +71,7 @@ describe('URL', () => {
7171

7272
const html = await text();
7373
assert.equal(html.includes('https:'), true);
74+
7475
});
7576

7677
it('includes forwarded host and port in the url', async () => {
@@ -132,7 +133,7 @@ describe('URL', () => {
132133
const $ = cheerio.load(html);
133134

134135
// Should use the Host header, not X-Forwarded-Host when allowedDomains is not configured
135-
assert.equal($('body').text(), 'https://legitimate.example.com/');
136+
assert.equal($('body').text(), 'http://legitimate.example.com/');
136137
});
137138

138139
it('rejects port in forwarded host when port not in allowedDomains', async () => {
@@ -153,7 +154,7 @@ describe('URL', () => {
153154
const $ = cheerio.load(html);
154155

155156
// Port 8080 not in allowedDomains (only 444), so should fall back to Host header
156-
assert.equal($('body').text(), 'https://localhost:3000/');
157+
assert.equal($('body').text(), 'http://localhost:3000/');
157158
});
158159

159160
it('rejects empty X-Forwarded-Host with allowedDomains configured', async () => {
@@ -174,7 +175,7 @@ describe('URL', () => {
174175
const $ = cheerio.load(html);
175176

176177
// Empty X-Forwarded-Host should be rejected and fall back to Host header
177-
assert.equal($('body').text(), 'https://legitimate.example.com/');
178+
assert.equal($('body').text(), 'http://legitimate.example.com/');
178179
});
179180

180181
it('rejects X-Forwarded-Host with path injection attempt', async () => {
@@ -195,6 +196,6 @@ describe('URL', () => {
195196
const $ = cheerio.load(html);
196197

197198
// Path injection attempt should be rejected and fall back to Host header
198-
assert.equal($('body').text(), 'https://localhost:3000/');
199+
assert.equal($('body').text(), 'http://localhost:3000/');
199200
});
200201
});

0 commit comments

Comments
 (0)