From 89bb9727a50a9cf5fca7afdbebabeb2591329c57 Mon Sep 17 00:00:00 2001 From: Nikita Tkalenko Date: Thu, 3 Oct 2024 17:48:55 +0300 Subject: [PATCH 1/4] fix(ilc): introduce test demonstrating open redirect vulnerability for case when locale is present --- ilc/common/UrlProcessor.spec.js | 12 ++++++++++++ ilc/server/tailor/fragment-hooks.js | 4 ++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/ilc/common/UrlProcessor.spec.js b/ilc/common/UrlProcessor.spec.js index e788d28d2..bb7a8482a 100644 --- a/ilc/common/UrlProcessor.spec.js +++ b/ilc/common/UrlProcessor.spec.js @@ -132,6 +132,18 @@ describe('UrlProcessor', () => { }); }); + it('when the path contains locale with multiple trailing slashes', () => { + const urlProcessor = new UrlProcessor(UrlProcessor.routerHasTo.redirectToTrailingSlash); + + const malformedUrl = '/de///google.com/'; + const expectedFixedUrl = '/de/google.com/'; + + chai.expect(urlProcessor.process(malformedUrl)).to.be.equal( + expectedFixedUrl, + `Failed for input: ${malformedUrl}`, + ); + }); + it('when the original provided URL does not have origin', () => { chai.expect( new UrlProcessor(UrlProcessor.routerHasTo.redirectToTrailingSlash).process( diff --git a/ilc/server/tailor/fragment-hooks.js b/ilc/server/tailor/fragment-hooks.js index 0440428e0..59638a98c 100644 --- a/ilc/server/tailor/fragment-hooks.js +++ b/ilc/server/tailor/fragment-hooks.js @@ -46,8 +46,8 @@ function insertStart(logger, stream, attributes, headers) { isAsync ? `` : id - ? asyncStylesLoadTemplate(uri, id) - : '', + ? asyncStylesLoadTemplate(uri, id) + : '', ); } else if (ref.rel === 'fragment-script') { bundleVersionOverrides.spaBundle = fixUri(attributes, ref.uri); From 62a23ee240e3189733a6b3623b894de449e0fde1 Mon Sep 17 00:00:00 2001 From: Nikita Tkalenko Date: Thu, 3 Oct 2024 18:05:08 +0300 Subject: [PATCH 2/4] fix(ilc): fix multiple trailing slash with open redirect --- ilc/common/UrlProcessor.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/ilc/common/UrlProcessor.js b/ilc/common/UrlProcessor.js index 2df41171f..1ddc5ac2b 100644 --- a/ilc/common/UrlProcessor.js +++ b/ilc/common/UrlProcessor.js @@ -29,29 +29,27 @@ 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); + + parsedUrl.pathname = parsedUrl.pathname.replace(/\/{2,}/g, '/'); + const doesUrlPathnameEndWithTrailingSlash = parsedUrl.pathname[parsedUrl.pathname.length - 1] === '/'; switch (this.#trailingSlash) { case UrlProcessor.routerHasTo.redirectToNonTrailingSlash: { if (doesUrlPathnameEndWithTrailingSlash) { parsedUrl.pathname = parsedUrl.pathname.slice(0, -1); - break; - } else { - return url; } + break; } case UrlProcessor.routerHasTo.redirectToTrailingSlash: { if (!doesUrlPathnameEndWithTrailingSlash) { parsedUrl.pathname = parsedUrl.pathname.concat('/'); - break; - } else { - return url; } + break; } } return parsedUrl.toString().replace(this.#fakeBaseInCasesWhereUrlIsRelative, ''); From d306208c186b9c644e7128ac9861382aa53b9d61 Mon Sep 17 00:00:00 2001 From: Nikita Tkalenko Date: Fri, 4 Oct 2024 10:41:16 +0300 Subject: [PATCH 3/4] fix(ilc): run prettier after updating node_moduels --- ilc/server/tailor/fragment-hooks.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ilc/server/tailor/fragment-hooks.js b/ilc/server/tailor/fragment-hooks.js index 59638a98c..0440428e0 100644 --- a/ilc/server/tailor/fragment-hooks.js +++ b/ilc/server/tailor/fragment-hooks.js @@ -46,8 +46,8 @@ function insertStart(logger, stream, attributes, headers) { isAsync ? `` : id - ? asyncStylesLoadTemplate(uri, id) - : '', + ? asyncStylesLoadTemplate(uri, id) + : '', ); } else if (ref.rel === 'fragment-script') { bundleVersionOverrides.spaBundle = fixUri(attributes, ref.uri); From 7bb93fb725567f116be3547706c0bc21fa2ac020 Mon Sep 17 00:00:00 2001 From: Nikita Tkalenko Date: Fri, 4 Oct 2024 10:44:13 +0300 Subject: [PATCH 4/4] fix(ilc): bring back comments --- ilc/common/UrlProcessor.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ilc/common/UrlProcessor.js b/ilc/common/UrlProcessor.js index 1ddc5ac2b..ddbd0ea87 100644 --- a/ilc/common/UrlProcessor.js +++ b/ilc/common/UrlProcessor.js @@ -29,11 +29,13 @@ 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); + // Ensure there are no double slashes in the pathname, excluding the start parsedUrl.pathname = parsedUrl.pathname.replace(/\/{2,}/g, '/'); const doesUrlPathnameEndWithTrailingSlash = parsedUrl.pathname[parsedUrl.pathname.length - 1] === '/';