From 258d5e2bfc2b3bc15cde4382e3a69e7a99aba113 Mon Sep 17 00:00:00 2001 From: Evgeniy Moroz Date: Tue, 11 Jul 2023 16:56:31 +0100 Subject: [PATCH 1/2] fix: open redirect vulnerability in case of always adding trailing slash. --- ilc/common/UrlProcessor.js | 9 ++++++++- ilc/common/UrlProcessor.spec.js | 10 ++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/ilc/common/UrlProcessor.js b/ilc/common/UrlProcessor.js index edec9123..6a478fdf 100644 --- a/ilc/common/UrlProcessor.js +++ b/ilc/common/UrlProcessor.js @@ -1,4 +1,6 @@ class UrlProcessor { + static validSchemes = ['http://', 'https://']; + static routerHasTo = { doNothing: 'doNothing', redirectToNonTrailingSlash: 'redirectToNonTrailingSlash', @@ -26,6 +28,11 @@ class UrlProcessor { } #processUrlTrailingSlash = (url) => { + if (!UrlProcessor.validSchemes.some((scheme) => url.startsWith(scheme))) { + // any combination of slashes at the beginning URL class classifies as an absolute link with http: protocol, so we remove them at the end + url = url.replace(/^[\/\\]+/gi, '/'); + } + const parsedUrl = new URL(url, this.#fakeBaseInCasesWhereUrlIsRelative); const doesUrlPathnameEndWithTrailingSlash = parsedUrl.pathname[parsedUrl.pathname.length - 1] === '/'; @@ -47,7 +54,7 @@ class UrlProcessor { } } } - + // throw new Error(parsedUrl.toString()) return parsedUrl.toString().replace(this.#fakeBaseInCasesWhereUrlIsRelative, ''); }; } diff --git a/ilc/common/UrlProcessor.spec.js b/ilc/common/UrlProcessor.spec.js index 193579e1..e788d28d 100644 --- a/ilc/common/UrlProcessor.spec.js +++ b/ilc/common/UrlProcessor.spec.js @@ -122,6 +122,16 @@ describe('UrlProcessor', () => { describe('when a router should redirect to an URL with trailing slash', () => { describe('should return an provided URL if it is with trailing slash at the end', () => { + it('when the path starts from double slash, which is an absolute URL for URL class', () => { + const expected = '/google.com/%2f/'; + const variants = ['\\//google.com/%2f', '//google.com/%2f', '///google.com/%2f', '/////google.com/%2f']; + const urlProcessor = new UrlProcessor(UrlProcessor.routerHasTo.redirectToTrailingSlash); + + variants.forEach((input) => { + chai.expect(urlProcessor.process(input)).to.be.equal(expected, `failed for ${input}`); + }); + }); + it('when the original provided URL does not have origin', () => { chai.expect( new UrlProcessor(UrlProcessor.routerHasTo.redirectToTrailingSlash).process( From d269f556068ace76a66e6906124610cafd93bba9 Mon Sep 17 00:00:00 2001 From: Evgeniy Moroz Date: Tue, 11 Jul 2023 16:58:10 +0100 Subject: [PATCH 2/2] chore: cleanup bad commit after debugging. --- ilc/common/UrlProcessor.js | 1 - 1 file changed, 1 deletion(-) diff --git a/ilc/common/UrlProcessor.js b/ilc/common/UrlProcessor.js index 6a478fdf..2df41171 100644 --- a/ilc/common/UrlProcessor.js +++ b/ilc/common/UrlProcessor.js @@ -54,7 +54,6 @@ class UrlProcessor { } } } - // throw new Error(parsedUrl.toString()) return parsedUrl.toString().replace(this.#fakeBaseInCasesWhereUrlIsRelative, ''); }; }