From 5f54067f6effe667edb591f57b218409dbdf328e Mon Sep 17 00:00:00 2001 From: Daniel Lopretto Date: Thu, 12 Nov 2020 19:15:21 -0500 Subject: [PATCH 1/6] Reproducing the Vulnerability --- test/unit/regression/SNYK-JS-AXIOS-1038255.js | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 test/unit/regression/SNYK-JS-AXIOS-1038255.js diff --git a/test/unit/regression/SNYK-JS-AXIOS-1038255.js b/test/unit/regression/SNYK-JS-AXIOS-1038255.js new file mode 100644 index 0000000000..a1dcec2eec --- /dev/null +++ b/test/unit/regression/SNYK-JS-AXIOS-1038255.js @@ -0,0 +1,49 @@ +// https://snyk.io/vuln/SNYK-JS-AXIOS-1038255 +// https://github.com/axios/axios/issues/3407 +// https://github.com/axios/axios/issues/3369 + +const axios = require('../../../index'); +const http = require('http'); +const assert = require('assert'); + +const PROXY_PORT = 4777; +const EVIL_PORT = 4666; + + +describe('Server-Side Request Forgery (SSRF)', () => { + let fail = false; + let proxy; + let server; + beforeEach(() => { + server = http.createServer(function (req, res) { + fail = true; + res.end('rm -rf /'); + }).listen(EVIL_PORT); + proxy = http.createServer(function (req, res) { + if (req.host === 'www.google.com') { + res.end('Googling'); + } + res.writeHead(302, {location: 'http://localhost:' + EVIL_PORT}) + res.end() + }).listen(PROXY_PORT); + }); + afterEach(() => { + server.close(); + proxy.close(); + }); + it('obeys proxy settings when following redirects', async () => { + let response = await axios({ + method: "get", + url: "http://www.google.com/", + proxy: { + host: "localhost", + port: PROXY_PORT, + }, + }); + + assert.strictEqual(fail, false); + assert.strictEqual(response.data, 'Googling'); + return response; + + }); +}); \ No newline at end of file From 8a90bcf8081c7e81bd7ff3ce1e427ce2244a6282 Mon Sep 17 00:00:00 2001 From: Daniel Lopretto Date: Thu, 12 Nov 2020 19:32:59 -0500 Subject: [PATCH 2/6] Prevent SSRF --- lib/adapters/http.js | 12 ++++++++++++ test/unit/regression/SNYK-JS-AXIOS-1038255.js | 6 +++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/lib/adapters/http.js b/lib/adapters/http.js index 135c93a35f..5c2013d826 100755 --- a/lib/adapters/http.js +++ b/lib/adapters/http.js @@ -175,6 +175,18 @@ module.exports = function httpAdapter(config) { options.maxBodyLength = config.maxBodyLength; } + if (proxy) { + // If a proxy is used any redirects must also pass through the proxy + options.beforeRedirect = function beforeRedirect(redirection) { + var location = redirection.href; + redirection.hostname = proxy.host; + redirection.host = proxy.host; + redirection.headers.host = parsed.hostname + (parsed.port ? ':' + parsed.port : ''); + redirection.port = proxy.port; + redirection.path = location; + }; + } + // Create the request var req = transport.request(options, function handleResponse(res) { if (req.aborted) return; diff --git a/test/unit/regression/SNYK-JS-AXIOS-1038255.js b/test/unit/regression/SNYK-JS-AXIOS-1038255.js index a1dcec2eec..507aac2a92 100644 --- a/test/unit/regression/SNYK-JS-AXIOS-1038255.js +++ b/test/unit/regression/SNYK-JS-AXIOS-1038255.js @@ -20,8 +20,8 @@ describe('Server-Side Request Forgery (SSRF)', () => { res.end('rm -rf /'); }).listen(EVIL_PORT); proxy = http.createServer(function (req, res) { - if (req.host === 'www.google.com') { - res.end('Googling'); + if (req.url === 'http://localhost:' + EVIL_PORT + '/') { + return res.end('Protected'); } res.writeHead(302, {location: 'http://localhost:' + EVIL_PORT}) res.end() @@ -42,7 +42,7 @@ describe('Server-Side Request Forgery (SSRF)', () => { }); assert.strictEqual(fail, false); - assert.strictEqual(response.data, 'Googling'); + assert.strictEqual(response.data, 'Protected'); return response; }); From db0404e14f2365827b03967f03049de0638c09b5 Mon Sep 17 00:00:00 2001 From: Daniel Lopretto Date: Thu, 12 Nov 2020 19:34:49 -0500 Subject: [PATCH 3/6] Cleanup --- lib/adapters/http.js | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/lib/adapters/http.js b/lib/adapters/http.js index 5c2013d826..a71bbb3a02 100755 --- a/lib/adapters/http.js +++ b/lib/adapters/http.js @@ -156,6 +156,16 @@ module.exports = function httpAdapter(config) { var base64 = Buffer.from(proxy.auth.username + ':' + proxy.auth.password, 'utf8').toString('base64'); options.headers['Proxy-Authorization'] = 'Basic ' + base64; } + + // If a proxy is used any redirects must also pass through the proxy + options.beforeRedirect = function beforeRedirect(redirection) { + var location = redirection.href; + redirection.hostname = proxy.host; + redirection.host = proxy.host; + redirection.headers.host = parsed.hostname + (parsed.port ? ':' + parsed.port : ''); + redirection.port = proxy.port; + redirection.path = location; + }; } var transport; @@ -175,18 +185,6 @@ module.exports = function httpAdapter(config) { options.maxBodyLength = config.maxBodyLength; } - if (proxy) { - // If a proxy is used any redirects must also pass through the proxy - options.beforeRedirect = function beforeRedirect(redirection) { - var location = redirection.href; - redirection.hostname = proxy.host; - redirection.host = proxy.host; - redirection.headers.host = parsed.hostname + (parsed.port ? ':' + parsed.port : ''); - redirection.port = proxy.port; - redirection.path = location; - }; - } - // Create the request var req = transport.request(options, function handleResponse(res) { if (req.aborted) return; From 2ec23411469506e0a9987a0b6a6a0f08bf8218a7 Mon Sep 17 00:00:00 2001 From: Daniel Lopretto Date: Thu, 12 Nov 2020 19:43:33 -0500 Subject: [PATCH 4/6] Refactor to skip duplicate code --- lib/adapters/http.js | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/lib/adapters/http.js b/lib/adapters/http.js index a71bbb3a02..0fc99f58a2 100755 --- a/lib/adapters/http.js +++ b/lib/adapters/http.js @@ -144,12 +144,12 @@ module.exports = function httpAdapter(config) { } } - if (proxy) { + function setProxy(options, proxy, location) { options.hostname = proxy.host; options.host = proxy.host; options.headers.host = parsed.hostname + (parsed.port ? ':' + parsed.port : ''); options.port = proxy.port; - options.path = protocol + '//' + parsed.hostname + (parsed.port ? ':' + parsed.port : '') + options.path; + options.path = location; // Basic proxy authorization if (proxy.auth) { @@ -159,13 +159,12 @@ module.exports = function httpAdapter(config) { // If a proxy is used any redirects must also pass through the proxy options.beforeRedirect = function beforeRedirect(redirection) { - var location = redirection.href; - redirection.hostname = proxy.host; - redirection.host = proxy.host; - redirection.headers.host = parsed.hostname + (parsed.port ? ':' + parsed.port : ''); - redirection.port = proxy.port; - redirection.path = location; + Object.assign(redirection, setProxy(redirection, proxy, redirection.href)); }; + return options; + } + if (proxy) { + options = setProxy(options, proxy, protocol + '//' + parsed.hostname + (parsed.port ? ':' + parsed.port : '') + options.path); } var transport; From 66cb23181672b4063ed48e7b821d82b45aba1888 Mon Sep 17 00:00:00 2001 From: Daniel Lopretto Date: Thu, 12 Nov 2020 20:42:53 -0500 Subject: [PATCH 5/6] Tests for correct passed data. --- lib/adapters/http.js | 47 ++++++----- test/unit/regression/SNYK-JS-AXIOS-1038255.js | 80 +++++++++++-------- 2 files changed, 74 insertions(+), 53 deletions(-) diff --git a/lib/adapters/http.js b/lib/adapters/http.js index 0fc99f58a2..c50c5e8b79 100755 --- a/lib/adapters/http.js +++ b/lib/adapters/http.js @@ -16,6 +16,33 @@ var enhanceError = require('../core/enhanceError'); var isHttps = /https:?/; +/** + * + * @param {http.ClientRequestArgs} options + * @param {AxiosProxyConfig} proxy + * @param {string} location + * @returns {http.ClientRequestArgs} + */ +function setProxy(options, proxy, location) { + options.hostname = proxy.host; + options.host = proxy.host; + options.port = proxy.port; + options.path = location; + + // Basic proxy authorization + if (proxy.auth) { + var base64 = Buffer.from(proxy.auth.username + ':' + proxy.auth.password, 'utf8').toString('base64'); + options.headers['Proxy-Authorization'] = 'Basic ' + base64; + } + + // If a proxy is used any redirects must also pass through the proxy + options.beforeRedirect = function beforeRedirect(redirection) { + redirection.headers.host = redirection.host; + Object.assign(redirection, setProxy(redirection, proxy, redirection.href)); + }; + return options; +} + /*eslint consistent-return:0*/ module.exports = function httpAdapter(config) { return new Promise(function dispatchHttpRequest(resolvePromise, rejectPromise) { @@ -144,26 +171,8 @@ module.exports = function httpAdapter(config) { } } - function setProxy(options, proxy, location) { - options.hostname = proxy.host; - options.host = proxy.host; - options.headers.host = parsed.hostname + (parsed.port ? ':' + parsed.port : ''); - options.port = proxy.port; - options.path = location; - - // Basic proxy authorization - if (proxy.auth) { - var base64 = Buffer.from(proxy.auth.username + ':' + proxy.auth.password, 'utf8').toString('base64'); - options.headers['Proxy-Authorization'] = 'Basic ' + base64; - } - - // If a proxy is used any redirects must also pass through the proxy - options.beforeRedirect = function beforeRedirect(redirection) { - Object.assign(redirection, setProxy(redirection, proxy, redirection.href)); - }; - return options; - } if (proxy) { + options.headers.host = parsed.hostname + (parsed.port ? ':' + parsed.port : ''); options = setProxy(options, proxy, protocol + '//' + parsed.hostname + (parsed.port ? ':' + parsed.port : '') + options.path); } diff --git a/test/unit/regression/SNYK-JS-AXIOS-1038255.js b/test/unit/regression/SNYK-JS-AXIOS-1038255.js index 507aac2a92..52c7498241 100644 --- a/test/unit/regression/SNYK-JS-AXIOS-1038255.js +++ b/test/unit/regression/SNYK-JS-AXIOS-1038255.js @@ -7,43 +7,55 @@ const http = require('http'); const assert = require('assert'); const PROXY_PORT = 4777; -const EVIL_PORT = 4666; +const EVIL_PORT = 4666; describe('Server-Side Request Forgery (SSRF)', () => { - let fail = false; - let proxy; - let server; - beforeEach(() => { - server = http.createServer(function (req, res) { - fail = true; - res.end('rm -rf /'); - }).listen(EVIL_PORT); - proxy = http.createServer(function (req, res) { - if (req.url === 'http://localhost:' + EVIL_PORT + '/') { - return res.end('Protected'); - } - res.writeHead(302, {location: 'http://localhost:' + EVIL_PORT}) - res.end() - }).listen(PROXY_PORT); + let fail = false; + let proxy; + let server; + let location; + beforeEach(() => { + server = http.createServer(function (req, res) { + fail = true; + res.end('rm -rf /'); + }).listen(EVIL_PORT); + proxy = http.createServer(function (req, res) { + if (req.url === 'http://localhost:' + EVIL_PORT + '/') { + return res.end(JSON.stringify({ + msg: 'Protected', + headers: req.headers, + })); + } + res.writeHead(302, { location }) + res.end() + }).listen(PROXY_PORT); + }); + afterEach(() => { + server.close(); + proxy.close(); + }); + it('obeys proxy settings when following redirects', async () => { + location = 'http://localhost:' + EVIL_PORT; + let response = await axios({ + method: "get", + url: "http://www.google.com/", + proxy: { + host: "localhost", + port: PROXY_PORT, + auth: { + username: 'sam', + password: 'password', + } + }, }); - afterEach(() => { - server.close(); - proxy.close(); - }); - it('obeys proxy settings when following redirects', async () => { - let response = await axios({ - method: "get", - url: "http://www.google.com/", - proxy: { - host: "localhost", - port: PROXY_PORT, - }, - }); - assert.strictEqual(fail, false); - assert.strictEqual(response.data, 'Protected'); - return response; - - }); + assert.strictEqual(fail, false); + assert.strictEqual(response.data.msg, 'Protected'); + assert.strictEqual(response.data.headers.host, 'localhost:' + EVIL_PORT); + assert.strictEqual(response.data.headers['proxy-authorization'], 'Basic ' + Buffer.from('sam:password').toString('base64')); + + return response; + + }); }); \ No newline at end of file From a500033c4f99bd9a591066095a7b6be8d51ebbb5 Mon Sep 17 00:00:00 2001 From: Daniel Lopretto Date: Mon, 23 Nov 2020 10:05:06 -0500 Subject: [PATCH 6/6] Code review changes. --- lib/adapters/http.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/adapters/http.js b/lib/adapters/http.js index c50c5e8b79..77174e2668 100755 --- a/lib/adapters/http.js +++ b/lib/adapters/http.js @@ -21,7 +21,6 @@ var isHttps = /https:?/; * @param {http.ClientRequestArgs} options * @param {AxiosProxyConfig} proxy * @param {string} location - * @returns {http.ClientRequestArgs} */ function setProxy(options, proxy, location) { options.hostname = proxy.host; @@ -35,12 +34,11 @@ function setProxy(options, proxy, location) { options.headers['Proxy-Authorization'] = 'Basic ' + base64; } - // If a proxy is used any redirects must also pass through the proxy + // If a proxy is used, any redirects must also pass through the proxy options.beforeRedirect = function beforeRedirect(redirection) { redirection.headers.host = redirection.host; - Object.assign(redirection, setProxy(redirection, proxy, redirection.href)); + setProxy(redirection, proxy, redirection.href); }; - return options; } /*eslint consistent-return:0*/ @@ -173,7 +171,7 @@ module.exports = function httpAdapter(config) { if (proxy) { options.headers.host = parsed.hostname + (parsed.port ? ':' + parsed.port : ''); - options = setProxy(options, proxy, protocol + '//' + parsed.hostname + (parsed.port ? ':' + parsed.port : '') + options.path); + setProxy(options, proxy, protocol + '//' + parsed.hostname + (parsed.port ? ':' + parsed.port : '') + options.path); } var transport;