Skip to content

Commit

Permalink
fix(sec): disregard protocol-relative URL to remediate SSRF (axios#6539)
Browse files Browse the repository at this point in the history
* fix(sec): disregard protocol-relative URL to remediate SSRF

Signed-off-by: hainenber <[email protected]>

* feat(test/unit/regression): add regression test to ensure SNYK-JS-AXIOS-7361793 fixed in future version

Signed-off-by: hainenber <[email protected]>

* chore: add EoF newline + comments

Signed-off-by: hainenber <[email protected]>

* chore: fix eslint issues

Signed-off-by: hainenber <[email protected]>

* Update SNYK-JS-AXIOS-7361793.js

Co-authored-by: tom-reinders <[email protected]>

---------

Signed-off-by: hainenber <[email protected]>
Co-authored-by: tom-reinders <[email protected]>
  • Loading branch information
hainenber and tom-reinders authored Aug 13, 2024
1 parent c6cce43 commit 07a661a
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 4 deletions.
4 changes: 2 additions & 2 deletions lib/helpers/isAbsoluteURL.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
* @returns {boolean} True if the specified URL is absolute, otherwise false
*/
export default function isAbsoluteURL(url) {
// A URL is considered absolute if it begins with "<scheme>://" or "//" (protocol-relative URL).
// A URL is considered absolute if it begins with "<scheme>://".
// RFC 3986 defines scheme name as a sequence of characters beginning with a letter and followed
// by any combination of letters, digits, plus, period, or hyphen.
return /^([a-z][a-z\d+\-.]*:)?\/\//i.test(url);
return /^([a-z][a-z\d+\-.]*:)\/\//i.test(url);
}
4 changes: 2 additions & 2 deletions test/specs/helpers/isAbsoluteURL.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ describe('helpers::isAbsoluteURL', function () {
expect(isAbsoluteURL('!valid://example.com/')).toBe(false);
});

it('should return true if URL is protocol-relative', function () {
expect(isAbsoluteURL('//example.com/')).toBe(true);
it('should return false if URL is protocol-relative', function () {
expect(isAbsoluteURL('//example.com/')).toBe(false);
});

it('should return false if URL is relative', function () {
Expand Down
45 changes: 45 additions & 0 deletions test/unit/regression/SNYK-JS-AXIOS-7361793.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// https://security.snyk.io/vuln/SNYK-JS-AXIOS-7361793
// https://github.com/axios/axios/issues/6463

import axios from '../../../index.js';
import http from 'http';
import assert from 'assert';

const GOOD_PORT = 4666;
const BAD_PORT = 4667;

describe('Server-Side Request Forgery (SSRF)', () => {
let goodServer, badServer;

beforeEach(() => {
goodServer = http.createServer(function (req, res) {
res.write('good');
res.end();
}).listen(GOOD_PORT);
badServer = http.createServer(function (req, res) {
res.write('bad');
res.end();
}).listen(BAD_PORT);
})

afterEach(() => {
goodServer.close();
badServer.close();
});

it('should not fetch bad server', async () => {
const ssrfAxios = axios.create({
baseURL: 'http://localhost:' + String(GOOD_PORT),
});

// Good payload would be `userId = '12345'`
// Malicious payload is as below.
const userId = '/localhost:' + String(BAD_PORT);

const response = await ssrfAxios.get(`/${userId}`);
assert.strictEqual(response.data, 'good');
assert.strictEqual(response.config.baseURL, 'http://localhost:' + String(GOOD_PORT));
assert.strictEqual(response.config.url, '//localhost:' + String(BAD_PORT));
assert.strictEqual(response.request.res.responseUrl, 'http://localhost:' + String(GOOD_PORT) + '/localhost:' + String(BAD_PORT));
});
});

0 comments on commit 07a661a

Please sign in to comment.