fix: open redirect vulnerability in case of always adding trailing sl…

…ash.
namecheap · Jul 11, 2023 · 258d5e2 · 258d5e2
1 parent 52c9ad8
commit 258d5e2
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 1 deletion.
diff --git 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
@@ -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(