From 6368f869c68563aba72052c91899a1f43fbcf2d4 Mon Sep 17 00:00:00 2001 From: Nathan Rajlich Date: Wed, 3 Jun 2020 18:35:09 -0700 Subject: [PATCH 1/2] Prevent open redirects when `cleanUrls` config is enabled --- src/index.js | 17 +++++++---------- test/integration.js | 14 ++++++++++++++ 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/index.js b/src/index.js index 85bbe76..85ea8df 100644 --- a/src/index.js +++ b/src/index.js @@ -127,15 +127,19 @@ const shouldRedirect = (decodedPath, {redirects = [], trailingSlash}, cleanUrl) return null; } - let cleanedUrl = false; - // By stripping the HTML parts from the decoded // path *before* handling the trailing slash, we make // sure that only *one* redirect occurs if both // config options are used. if (cleanUrl && matchHTML.test(decodedPath)) { decodedPath = decodedPath.replace(matchHTML, ''); - cleanedUrl = true; + if (decodedPath.indexOf('//') > -1) { + decodedPath = decodedPath.replace(/\/+/g, '/'); + } + return { + target: ensureSlashStart(decodedPath), + statusCode: defaultType + }; } if (slashing) { @@ -163,13 +167,6 @@ const shouldRedirect = (decodedPath, {redirects = [], trailingSlash}, cleanUrl) } } - if (cleanedUrl) { - return { - target: ensureSlashStart(decodedPath), - statusCode: defaultType - }; - } - // This is currently the fastest way to // iterate over an array for (let index = 0; index < redirects.length; index++) { diff --git a/test/integration.js b/test/integration.js index 61b812b..18a1511 100644 --- a/test/integration.js +++ b/test/integration.js @@ -278,6 +278,20 @@ test('set `trailingSlash` config property to `false`', async t => { t.is(location, target); }); +test('set `cleanUrls` config property should prevent open redirects', async t => { + const url = await getUrl({ + cleanUrls: true + }); + + const response = await fetch(`${url}//haveibeenpwned.com/index`, { + redirect: 'manual', + follow: 0 + }); + + const location = response.headers.get('location'); + t.is(location, `${url}/haveibeenpwned.com`); +}); + test('set `rewrites` config property to wildcard path', async t => { const destination = '.dotfile'; const related = path.join(fixturesFull, destination); From 07615875693746333510d6bb36163a376a453528 Mon Sep 17 00:00:00 2001 From: Nathan Rajlich Date: Wed, 3 Jun 2020 19:10:55 -0700 Subject: [PATCH 2/2] Cov --- src/index.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/index.js b/src/index.js index 85ea8df..05e3430 100644 --- a/src/index.js +++ b/src/index.js @@ -409,6 +409,7 @@ const renderDirectory = async (current, acceptsJSON, handlers, methods, config, return 1; } + /* istanbul ignore next */ if (a.base < b.base) { return -1; }