Skip to content

Commit

Permalink
Merge pull request #520 from namecheap/feature/fix-open-redirect
Browse files Browse the repository at this point in the history
fix: open redirect vulnerability in case of always adding trailing slash
  • Loading branch information
b1ff authored Jul 11, 2023
2 parents 52c9ad8 + d269f55 commit 1b9c709
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 1 deletion.
8 changes: 7 additions & 1 deletion ilc/common/UrlProcessor.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
class UrlProcessor {
static validSchemes = ['http://', 'https://'];

static routerHasTo = {
doNothing: 'doNothing',
redirectToNonTrailingSlash: 'redirectToNonTrailingSlash',
Expand Down Expand Up @@ -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] === '/';

Expand All @@ -47,7 +54,6 @@ class UrlProcessor {
}
}
}

return parsedUrl.toString().replace(this.#fakeBaseInCasesWhereUrlIsRelative, '');
};
}
Expand Down
10 changes: 10 additions & 0 deletions ilc/common/UrlProcessor.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down

0 comments on commit 1b9c709

Please sign in to comment.