fix(platform-server): prevent SSRF bypasses via backslash URLs in HttpClient#68845
Open
JeanMeche wants to merge 2 commits into
Open
fix(platform-server): prevent SSRF bypasses via backslash URLs in HttpClient#68845JeanMeche wants to merge 2 commits into
JeanMeche wants to merge 2 commits into
Conversation
…pClient Encoding backslashes ensures that they are not normalized to slashes and where they could generate a protocol relative URL.
alan-agius4
requested changes
May 21, 2026
| }); | ||
| }); | ||
|
|
||
| it('prevents SSRF bypasses via backslash URLs in HttpClient', async () => { |
Contributor
There was a problem hiding this comment.
This test only covers a small part. Can you add some more included cases that should pass.
it('should allow legitimate protocol-relative URLs', async () => {
ref.injector.get(NgZone).run(() => {
http.get('//example.com/testing').subscribe((body) => {
expect(body).toEqual('success!');
});
mock.expectOne('http://example.com/testing').flush('success!');
});
});
it('should treat backslash bypass SSRF attempts in relative requests strictly as pathnames', async () => {
const badUrls = [
'/\\attacker.com',
'\\\\attacker.com',
'///attacker.com',
'//\\attacker.com',
' /\\attacker.com',
'\r\n/\\attacker.com',
];
ref.injector.get(NgZone).run(() => {
for (const badUrl of badUrls) {
http.get(badUrl).subscribe((body) => {
expect(body).toEqual('success!');
});
mock.expectOne('http://localhost:4000/attacker.com').flush('success!');
}
});
});
it('should resolve safe path-relative URLs containing backslashes without origin change', async () => {
ref.injector.get(NgZone).run(() => {
http.get('\\testing').subscribe((body) => {
expect(body).toEqual('success!');
});
mock.expectOne('http://localhost:4000/testing').flush('success!');
});
});
it('should resolve backslashes inside path-relative segments without origin change', async () => {
ref.injector.get(NgZone).run(() => {
http.get('/foo\\bar').subscribe((body) => {
expect(body).toEqual('success!');
});
mock.expectOne('http://localhost:4000/foo/bar').flush('success!');
});
});
it('should resolve relative request URLs without leading slash relative to parent path', async () => {
ref.injector.get(NgZone).run(() => {
http.get('testing').subscribe((body) => {
expect(body).toEqual('success!');
});
mock.expectOne('http://localhost:4000/testing').flush('success!');
});
});
});
describe(`given 'url' is provided in 'INITIAL_CONFIG' with a trailing slash`, () => {
let mock: HttpTestingController;
let ref: NgModuleRef<HttpInterceptorExampleModule>;
let http: HttpClient;
beforeEach(async () => {
const platform = platformServer([
{
provide: INITIAL_CONFIG,
useValue: {
document: '<app></app>',
url: 'http://localhost:4000/foo/',
},
},
]);
ref = await platform.bootstrapModule(HttpInterceptorExampleModule);
mock = ref.injector.get(HttpTestingController);
http = ref.injector.get(HttpClient);
});
it('should resolve sub-path relative request URLs relative to trailing-slash base URL', async () => {
ref.injector.get(NgZone).run(() => {
http.get('testing').subscribe((body) => {
expect(body).toEqual('success!');
});
mock.expectOne('http://localhost:4000/foo/testing').flush('success!');
});
});| // The `URL` constructor normalizes backslashes to forward slashes, which can | ||
| // cause `/\` to be evaluated as a protocol-relative URL (`//`). | ||
| // Replacing backslashes with their URL-encoded equivalent prevents this. | ||
| const safeUrl = request.url.replace(/\\/g, '%5C'); |
Contributor
There was a problem hiding this comment.
While this isn't a massive issue, it forces all backslashes to be encode, even those that aren't meant to be protocol-relative. For instance, an input like foo\\bar gets parsed into a pathname of /foo%5Cbar, which alters expected URL parsing behavior and could break legitimate uses.
Additionally, this doesn't catch forward slashes //, which are also treated as protocol-relative by new URL(). An input like //foo would still bypass this check and leave us vulnerable.
I think a better approach would look something like this:
// Standardizing backslashes to forward slashes ensures the URL constructor resolves it properly.
// This is the same behaviour of new URL
const originalUrl = request.url.trim();
let url = originalUrl.replace(/\\/g, '/');
// If it looks like a relative path, ensure it starts with a single forward slash.
// We check legitimacy on the originalUrl before backslash substitution, ensuring that relative bypass
// attempts (like '/\\' or '\\\\') are treated strictly as paths rather than being converted to protocol-relative targets.
const isAbsolute = URL.canParse(originalUrl);
const isLegitProtocolRelative = originalUrl.startsWith('//') && originalUrl[2] !== '/' && originalUrl[2] !== '\\';
if (!isAbsolute && !isLegitProtocolRelative) {
if (originalUrl[0] === '/' || originalUrl[0] === '\\') {
url = '/' + url.replace(/^\/+/, '');
}
}
const newUrl = new URL(url, baseUrl).toString();8e398e6 to
6611e71
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Encoding backslashes ensures that they are not normalized to slashes and where they could generate a protocol relative URL.