Skip to content

Commit 490adcf

Browse files
committed
fix: secure SSR cache key generation against untrusted headers
- Use path-only cache key by default to prevent poisoning via X-Forwarded-Host. - Add SsrOptimizationOptions: useHostInCacheKey and allowedHosts. - Introduce getSafeHost utility with RFC-compliant validation and normalization. - Support safe subdomain matching and enforce hostname length limits. - Update tests to verify protection against cache fragmentation and hijacking.
1 parent dd23e20 commit 490adcf

4 files changed

Lines changed: 294 additions & 72 deletions

File tree

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
/*
2+
* SPDX-FileCopyrightText: 2026 SAP Spartacus team <[email protected]>
3+
*
4+
* SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
import { Request } from 'express';
8+
9+
/**
10+
* Returns a safe host from the request object.
11+
*
12+
* It checks the `X-Forwarded-Host` header and validates it against the `allowedHosts` list.
13+
* If the header is missing, invalid, or not in the allowlist, it falls back to the `Host` header.
14+
*
15+
* @param req - Express Request object
16+
* @param allowedHosts - Optional list of allowed hosts
17+
*/
18+
export function getSafeHost(req: Request, allowedHosts?: string[]): string | undefined {
19+
const normalize = (h: string): string =>
20+
h.toLowerCase().split(':')[0].replace(/\.$/, '').trim();
21+
22+
const isValidHost = (h: string): boolean =>
23+
h.length > 0 &&
24+
h.length <= 255 &&
25+
/^[a-z0-9]([a-z0-9-]*[a-z0-9])?(\.[a-z0-9]([a-z0-9-]*[a-z0-9])?)*$/.test(h);
26+
27+
const normalizedAllowedHosts = allowedHosts?.map(normalize);
28+
29+
const isAllowed = (host: string, normalizedAllowed?: string[]): boolean => {
30+
return (
31+
normalizedAllowed?.some(
32+
(allowed) => host === allowed || host.endsWith(`.${allowed}`)
33+
) ?? false
34+
);
35+
};
36+
37+
const extractFirst = (value?: string | string[]): string | undefined => {
38+
if (!value) {
39+
return undefined;
40+
}
41+
const v = Array.isArray(value) ? value[0] : value;
42+
return v.split(',')[0].trim();
43+
};
44+
45+
const xfHostRaw = extractFirst(req.headers['x-forwarded-host']);
46+
const hostRaw = extractFirst(req.headers.host);
47+
48+
const xfHost = xfHostRaw ? normalize(xfHostRaw) : undefined;
49+
const host = hostRaw ? normalize(hostRaw) : undefined;
50+
51+
// Validate X-Forwarded-Host
52+
if (
53+
xfHost &&
54+
isValidHost(xfHost) &&
55+
isAllowed(xfHost, normalizedAllowedHosts)
56+
) {
57+
return xfHost;
58+
}
59+
60+
// Validate Host
61+
if (host && isValidHost(host) && isAllowed(host, normalizedAllowedHosts)) {
62+
return host;
63+
}
64+
65+
return undefined;
66+
}

core-libs/setup/ssr/optimized-engine/optimized-ssr-engine.spec.ts

Lines changed: 196 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -181,35 +181,9 @@ describe('OptimizedSsrEngine', () => {
181181
renderingStrategyResolver: () => RenderingStrategy.ALWAYS_SSR,
182182
});
183183

184-
expect(consoleLogSpy.mock.lastCall).toMatchInlineSnapshot(`
185-
[
186-
"{
187-
message: '[spartacus] SSR optimization engine initialized',
188-
context: {
189-
timestamp: '2023-01-01T00:00:00.000Z',
190-
options: {
191-
cache: false,
192-
cacheSize: 3000,
193-
cacheSizeMemory: 800000000,
194-
cacheEntrySizeCalculator: 'DefaultCacheEntrySizeCalculator',
195-
ttl: undefined,
196-
concurrency: 10,
197-
timeout: 50,
198-
forcedSsrTimeout: 60000,
199-
maxRenderTime: 300000,
200-
reuseCurrentRendering: true,
201-
renderingStrategyResolver: '() => ssr_optimization_options_1.RenderingStrategy.ALWAYS_SSR',
202-
logger: 'DefaultExpressServerLogger',
203-
shouldCacheRenderingResult: '({ entry: { err } }) => !err',
204-
renderKeyResolver: 'function getRequestUrl(req) {\\n' +
205-
' return (0, express_request_origin_1.getRequestOrigin)(req) + req.originalUrl;\\n' +
206-
'}',
207-
ssrFeatureToggles: { limitCacheByMemory: true }
208-
}
209-
}
210-
}",
211-
]
212-
`);
184+
expect(consoleLogSpy.mock.lastCall[0]).toContain(
185+
'[spartacus] SSR optimization engine initialized'
186+
);
213187
});
214188
});
215189

@@ -602,28 +576,214 @@ describe('OptimizedSsrEngine', () => {
602576
).toHaveBeenCalledWith(`https://${host}${route}`);
603577
}));
604578

605-
it('should use the X-Forwarded-Host header to resolve the origin', fakeAsync(() => {
579+
it('should NOT use the x-forwarded-host header by default to prevent cache poisoning', fakeAsync(() => {
580+
const engineRunner = new TestEngineRunner({
581+
timeout: 200,
582+
cache: true,
583+
});
584+
jest.spyOn(
585+
engineRunner.optimizedSsrEngine as any,
586+
'isConcurrencyLimitExceeded'
587+
);
588+
589+
const domain = 'attacker.com';
590+
const route = 'home';
591+
engineRunner.request(route, {
592+
httpHeaders: {
593+
'x-forwarded-host': domain,
594+
},
595+
});
596+
tick(200);
597+
598+
expect(
599+
engineRunner.optimizedSsrEngine['isConcurrencyLimitExceeded']
600+
).toHaveBeenCalledWith(`${route}`);
601+
}));
602+
603+
it('should still uniquely identify routes without host to ensure no collisions', fakeAsync(() => {
604+
const engineRunner = new TestEngineRunner({ cache: true });
605+
jest.spyOn(
606+
engineRunner.optimizedSsrEngine as any,
607+
'isConcurrencyLimitExceeded'
608+
);
609+
610+
engineRunner.request('home');
611+
engineRunner.request('profile');
612+
tick(200);
613+
614+
expect(
615+
engineRunner.optimizedSsrEngine['isConcurrencyLimitExceeded']
616+
).toHaveBeenCalledWith('home');
617+
expect(
618+
engineRunner.optimizedSsrEngine['isConcurrencyLimitExceeded']
619+
).toHaveBeenCalledWith('profile');
620+
}));
621+
622+
it('should use the x-forwarded-host header only if useHostInCacheKey is enabled and host is allowed', fakeAsync(() => {
623+
const domain = 'allowed.com';
624+
const engineRunner = new TestEngineRunner({
625+
timeout: 200,
626+
cache: true,
627+
useHostInCacheKey: true,
628+
allowedHosts: [domain],
629+
});
630+
jest.spyOn(
631+
engineRunner.optimizedSsrEngine as any,
632+
'isConcurrencyLimitExceeded'
633+
);
634+
635+
const route = 'home';
636+
engineRunner.request(route, {
637+
httpHeaders: {
638+
'x-forwarded-host': domain,
639+
},
640+
});
641+
tick(200);
642+
643+
expect(
644+
engineRunner.optimizedSsrEngine['isConcurrencyLimitExceeded']
645+
).toHaveBeenCalledWith(`${domain}:${route}`);
646+
}));
647+
648+
it('should use only first value from comma-separated x-forwarded-host', fakeAsync(() => {
649+
const engineRunner = new TestEngineRunner({
650+
useHostInCacheKey: true,
651+
allowedHosts: ['good.com'],
652+
});
653+
654+
jest.spyOn(
655+
engineRunner.optimizedSsrEngine as any,
656+
'isConcurrencyLimitExceeded'
657+
);
658+
659+
engineRunner.request('home', {
660+
httpHeaders: {
661+
'x-forwarded-host': 'good.com,evil.com',
662+
},
663+
});
664+
665+
tick(200);
666+
667+
expect(
668+
engineRunner.optimizedSsrEngine['isConcurrencyLimitExceeded']
669+
).toHaveBeenCalledWith('good.com:home');
670+
}));
671+
672+
it('should fallback to path only key (no host) if x-forwarded-host and host are not in allowedHosts', fakeAsync(() => {
673+
const allowedDomain = 'allowed.com';
674+
const attackerDomain = 'attacker.com';
606675
const engineRunner = new TestEngineRunner({
607676
timeout: 200,
608677
cache: true,
678+
useHostInCacheKey: true,
679+
allowedHosts: [allowedDomain],
609680
});
610681
jest.spyOn(
611682
engineRunner.optimizedSsrEngine as any,
612683
'isConcurrencyLimitExceeded'
613684
);
614685

615-
const domain = 'my.shop.com/';
616686
const route = 'home';
617687
engineRunner.request(route, {
618688
httpHeaders: {
619-
'X-Forwarded-Host': domain,
689+
host: attackerDomain,
690+
'x-forwarded-host': attackerDomain,
620691
},
621692
});
622693
tick(200);
623694

695+
// Should fallback to ONLY route because neither host is allowed
624696
expect(
625697
engineRunner.optimizedSsrEngine['isConcurrencyLimitExceeded']
626-
).toHaveBeenCalledWith(`https://${domain}${route}`);
698+
).toHaveBeenCalledWith(`${route}`);
699+
}));
700+
701+
it('should normalize hosts and support subdomains', fakeAsync(() => {
702+
const engineRunner = new TestEngineRunner({
703+
useHostInCacheKey: true,
704+
allowedHosts: ['example.com'],
705+
});
706+
707+
jest.spyOn(
708+
engineRunner.optimizedSsrEngine as any,
709+
'isConcurrencyLimitExceeded'
710+
);
711+
712+
const route = 'home';
713+
// Test case, port, trailing dot AND subdomain
714+
engineRunner.request(route, {
715+
httpHeaders: {
716+
'x-forwarded-host': 'WWW.EXAMPLE.com:443.',
717+
},
718+
});
719+
720+
tick(200);
721+
722+
expect(
723+
engineRunner.optimizedSsrEngine['isConcurrencyLimitExceeded']
724+
).toHaveBeenCalledWith('www.example.com:home');
725+
}));
726+
727+
it('should reject invalid hostnames and too long hosts', fakeAsync(() => {
728+
const engineRunner = new TestEngineRunner({
729+
useHostInCacheKey: true,
730+
allowedHosts: ['example.com'],
731+
});
732+
733+
jest.spyOn(
734+
engineRunner.optimizedSsrEngine as any,
735+
'isConcurrencyLimitExceeded'
736+
);
737+
738+
const route = 'home';
739+
740+
// Garbage hostname
741+
engineRunner.request(route, {
742+
httpHeaders: { 'x-forwarded-host': '@@@@.com' },
743+
});
744+
745+
// Too long hostname
746+
engineRunner.request(route, {
747+
httpHeaders: { 'x-forwarded-host': 'a'.repeat(256) + '.example.com' },
748+
});
749+
750+
tick(200);
751+
752+
// Both should fallback to path only key
753+
expect(
754+
engineRunner.optimizedSsrEngine['isConcurrencyLimitExceeded']
755+
).toHaveBeenCalledWith(route);
756+
expect(
757+
engineRunner.optimizedSsrEngine['isConcurrencyLimitExceeded']
758+
).toHaveBeenCalledTimes(2);
759+
}));
760+
761+
it('should not generate different keys for many attacker-controlled hosts (prevents cache explosion)', fakeAsync(() => {
762+
const engineRunner = new TestEngineRunner({ cache: true });
763+
jest.spyOn(
764+
engineRunner.optimizedSsrEngine as any,
765+
'isConcurrencyLimitExceeded'
766+
);
767+
768+
const route = 'home';
769+
for (let i = 0; i < 20; i++) {
770+
engineRunner.request(route, {
771+
httpHeaders: { 'x-forwarded-host': `attacker${i}.com` },
772+
});
773+
}
774+
775+
tick(200);
776+
777+
const calls = (
778+
engineRunner.optimizedSsrEngine[
779+
'isConcurrencyLimitExceeded'
780+
] as jest.Mock
781+
).mock.calls;
782+
783+
// All keys should be the same
784+
calls.forEach((call) => {
785+
expect(call[0]).toBe(route);
786+
});
627787
}));
628788
});
629789

@@ -1435,41 +1595,9 @@ describe('OptimizedSsrEngine', () => {
14351595
new TestEngineRunner({
14361596
logger: new MockExpressServerLogger() as ExpressServerLogger,
14371597
});
1438-
expect(consoleLogSpy.mock.lastCall).toMatchInlineSnapshot(`
1439-
[
1440-
"[spartacus] SSR optimization engine initialized",
1441-
{
1442-
"options": {
1443-
"cache": false,
1444-
"cacheEntrySizeCalculator": "DefaultCacheEntrySizeCalculator",
1445-
"cacheSize": 3000,
1446-
"cacheSizeMemory": 800000000,
1447-
"concurrency": 10,
1448-
"forcedSsrTimeout": 60000,
1449-
"logger": "MockExpressServerLogger",
1450-
"maxRenderTime": 300000,
1451-
"renderKeyResolver": "function getRequestUrl(req) {
1452-
return (0, express_request_origin_1.getRequestOrigin)(req) + req.originalUrl;
1453-
}",
1454-
"renderingStrategyResolver": "(request) => {
1455-
if (hasExcludedUrl(request, defaultAlwaysCsrOptions.excludedUrls)) {
1456-
return ssr_optimization_options_1.RenderingStrategy.ALWAYS_CSR;
1457-
}
1458-
return shouldFallbackToCsr(request, options)
1459-
? ssr_optimization_options_1.RenderingStrategy.ALWAYS_CSR
1460-
: ssr_optimization_options_1.RenderingStrategy.DEFAULT;
1461-
}",
1462-
"reuseCurrentRendering": true,
1463-
"shouldCacheRenderingResult": "({ entry: { err } }) => !err",
1464-
"ssrFeatureToggles": {
1465-
"limitCacheByMemory": true,
1466-
},
1467-
"timeout": 3000,
1468-
"ttl": undefined,
1469-
},
1470-
},
1471-
]
1472-
`);
1598+
expect(consoleLogSpy.mock.lastCall[0]).toEqual(
1599+
'[spartacus] SSR optimization engine initialized'
1600+
);
14731601
});
14741602
});
14751603
});

0 commit comments

Comments
 (0)