From a9e06a9dabc6648dbf1dcac3690d8ece205fa8d3 Mon Sep 17 00:00:00 2001 From: Rob Wu Date: Wed, 6 May 2020 23:25:17 +0200 Subject: [PATCH] Reject invalid redirects Fixes #234. --- lib/cors-anywhere.js | 7 +++++-- test/setup.js | 5 +++++ test/test.js | 13 +++++++++++++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/lib/cors-anywhere.js b/lib/cors-anywhere.js index 7fd8ec80..c9d8936c 100644 --- a/lib/cors-anywhere.js +++ b/lib/cors-anywhere.js @@ -171,9 +171,12 @@ function onProxyResponse(proxy, proxyReq, proxyRes, req, res) { // Handle redirects if (statusCode === 301 || statusCode === 302 || statusCode === 303 || statusCode === 307 || statusCode === 308) { var locationHeader = proxyRes.headers.location; + var parsedLocation; if (locationHeader) { locationHeader = url.resolve(requestState.location.href, locationHeader); - + parsedLocation = parseURL(locationHeader); + } + if (parsedLocation) { if (statusCode === 301 || statusCode === 302 || statusCode === 303) { // Exclude 307 & 308, because they are rare, and require preserving the method + request body requestState.redirectCount_ = requestState.redirectCount_ + 1 || 1; @@ -186,7 +189,7 @@ function onProxyResponse(proxy, proxyReq, proxyRes, req, res) { req.method = 'GET'; req.headers['content-length'] = '0'; delete req.headers['content-type']; - requestState.location = parseURL(locationHeader); + requestState.location = parsedLocation; // Remove all listeners (=reset events to initial state) req.removeAllListeners(); diff --git a/test/setup.js b/test/setup.js index 011b577a..61a1d019 100644 --- a/test/setup.js +++ b/test/setup.js @@ -114,6 +114,11 @@ nock('http://example.com') .get('/redirectwithoutlocation') .reply(302, 'maybe found') + .get('/redirectinvalidlocation') + .reply(302, 'redirecting to junk...', { + Location: 'http:///', + }) + .get('/proxyerror') .replyWithError('throw node') ; diff --git a/test/test.js b/test/test.js index 46f8949e..95f2fb60 100644 --- a/test/test.js +++ b/test/test.js @@ -246,6 +246,19 @@ describe('Basic functionality', function() { .expect(302, 'maybe found', done); }); + it('GET with 302 redirect to an invalid Location should not be followed', function(done) { + // There is nothing to follow, so let the browser decide what to do with it. + request(cors_anywhere) + .get('/example.com/redirectinvalidlocation') + .redirects(0) + .expect('Access-Control-Allow-Origin', '*') + .expect('x-request-url', 'http://example.com/redirectinvalidlocation') + .expect('x-final-url', 'http://example.com/redirectinvalidlocation') + .expect('access-control-expose-headers', /x-final-url/) + .expect('Location', 'http:///') + .expect(302, 'redirecting to junk...', done); + }); + it('POST with 307 redirect should not be handled', function(done) { // Because of implementation difficulties (having to keep the request body // in memory), handling HTTP 307/308 redirects is deferred to the requestor.