Merge pull request #628 from namecheap/fix/another-openredirect-case

fix: another case of open redirect

ilc/common/UrlProcessor.js

@@ -54,7 +54,13 @@ class UrlProcessor {
                 break;
             }
         }
-        return parsedUrl.toString().replace(this.#fakeBaseInCasesWhereUrlIsRelative, '');
+
+        let resultUrl = parsedUrl.toString().replace(this.#fakeBaseInCasesWhereUrlIsRelative, '');
+
+        // Strip any trailing double slashes from the pathname, if present
+        resultUrl = resultUrl.replace(/\/{2,}$/g, '/');
+
+        return resultUrl;
     };
 }
 

ilc/common/UrlProcessor.spec.js

@@ -201,6 +201,15 @@ describe('UrlProcessor', () => {
                     new UrlProcessor(UrlProcessor.routerHasTo.redirectToTrailingSlash).process(urlWithQueryAndHash),
                 ).to.be.equal(urlWithQueryAndHashAndTrailingSlashAtTheEnd);
             });
+
+            it('when the path starts with multiple slashes and ends with multiple slashes', () => {
+                const input = 'https://original.url//google.com//';
+                const expected = 'https://original.url/google.com/';
+
+                const urlProcessor = new UrlProcessor(UrlProcessor.routerHasTo.redirectToTrailingSlash);
+
+                chai.expect(urlProcessor.process(input)).to.be.equal(expected, `Failed for input: ${input}`);
+            });
         });
     });
 });

ilc/server/app.spec.js

@@ -77,5 +77,17 @@ describe('App', () => {
 
             chai.expect(response.headers.location).to.be.eql('/someRoute/');
         });
+
+        it('should reply with 302 in case of open redirect attempt but lead to correct route', async () => {
+            const response = await server.get('//google.com//').expect(302);
+
+            chai.expect(response.headers.location).to.be.eql('/google.com/');
+        });
+
+        it('should reply with 302 in case of open redirect attempt but lead to correct route for complex route', async () => {
+            const response = await server.get('//google.com//someRoute//').expect(302);
+
+            chai.expect(response.headers.location).to.be.eql('/google.com/someRoute/');
+        });
     });
 });

ilc/server/i18n.js

@@ -42,16 +42,20 @@ const onRequestFactory =
         );
 
         if (!req.raw.url.startsWith('/_ilc/')) {
-            const fixedUrl = IlcIntl.localizeUrl(i18nConfig, req.raw.url, { locale: pluginProcessedI18nConfig.locale });
+            const localizedUrl = IlcIntl.localizeUrl(i18nConfig, req.raw.url, {
+                locale: pluginProcessedI18nConfig.locale,
+            });
 
             const containsOnlySlashes = (url) => /^\/+$/.test(url);
             const shouldSkipRedirectForSlashes =
                 registryTrailingSlashSetting === UrlProcessor.routerHasTo.doNothing &&
-                containsOnlySlashes(fixedUrl) &&
+                containsOnlySlashes(localizedUrl) &&
                 containsOnlySlashes(req.raw.url);
 
-            if (fixedUrl !== req.raw.url && !shouldSkipRedirectForSlashes) {
-                return reply.redirect(fixedUrl);
+            if (localizedUrl !== req.raw.url && !shouldSkipRedirectForSlashes) {
+                const urlProcessor = new UrlProcessor(registryTrailingSlashSetting);
+                // Even if the localization triggered redirect we still need to ensure it is open-redirect safe
+                return reply.redirect(urlProcessor.process(localizedUrl));
             }
         }