Skip to content

Commit

Permalink
fix UI open redirect vulnerability (AthenZ#700)
Browse files Browse the repository at this point in the history
  • Loading branch information
WindzCUHK authored and havetisyan committed Jun 7, 2019
1 parent 32b60a4 commit 247ed2e
Show file tree
Hide file tree
Showing 4 changed files with 144 additions and 9 deletions.
9 changes: 2 additions & 7 deletions ui/index.js
Expand Up @@ -26,6 +26,7 @@ var hbs = require('express-handlebars');

var pageUtils = require('./src/utils/page');
var hbsHelpers = require('./src/utils/handlebarHelpers');
var middleware = require('./src/utils/middleware.js');
var routeHandlers = require('./src/routeHandlers/main');
var searchRoutes = require('./src/routeHandlers/search');
var domainRoutes = require('./src/routeHandlers/domain');
Expand Down Expand Up @@ -84,13 +85,7 @@ var client = require('./src/rdl-rest.js')({

// middle-ware to redirect when there is trailing slash so that urls are
// guaranteed not to contain trailing slash
app.use(function(req, res, next) {
if (req.url.substr(-1) === '/' && req.url.length > 1) {
res.redirect(301, req.url.slice(0, -1));
} else {
next();
}
});
app.use(middleware.redirectOnTrailingSlash);

loginUtils.signUserToken(app);
loginUtils.authenticateUser(app);
Expand Down
4 changes: 2 additions & 2 deletions ui/package.json
Expand Up @@ -12,8 +12,8 @@
"scripts": {
"build": "make",
"lint": "grunt lint",
"test": "./node_modules/nyc/bin/nyc.js node_modules/.bin/jenkins-mocha --require test/config/mock.js test/unit/*.js",
"testDev": "mocha --require test/config/mock.js test/unit/*.js",
"test": "./node_modules/nyc/bin/nyc.js node_modules/.bin/jenkins-mocha --require test/config/mock.js test/unit/*.js test/unit/*/*.js",
"testDev": "mocha ./test/unit -name '*.js' --recursive",
"functional": "grunt functional-sd",
"noop": "echo Ignoring this section."
},
Expand Down
31 changes: 31 additions & 0 deletions ui/src/utils/middleware.js
@@ -0,0 +1,31 @@
/**
* Copyright 2016 Yahoo Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
'use strict';

const config = require('../../config/config.js')();

const redirectOnTrailingSlash = (req, res, next) => {
if (req.url.substr(-1) === '/' && req.url.length > 1) {
const domain = process.env.UI_SERVER || 'localhost';
res.redirect(301, '//' + domain + req.url.slice(0, -1));
} else {
next();
}
};

module.exports = {
redirectOnTrailingSlash,
};
109 changes: 109 additions & 0 deletions ui/test/unit/utils/middleware.js
@@ -0,0 +1,109 @@
/**
* Copyright 2016 Yahoo Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
'use strict';

const middleware = require('../../../src/utils/middleware.js');

const expect = require('chai').expect;
const sinon = require('sinon');

let sandbox;
describe('middleware utils', () => {

beforeEach(() => {
// sandbox = sinon.createSandbox();
sandbox = sinon.sandbox.create();
});
afterEach(() => {
sandbox.restore();
});

describe('redirectOnTrailingSlash', () => {

let req, res, next;
beforeEach(() => {
req = {};
res = {};
res.redirect = sandbox.spy();
next = sandbox.spy();
});

const restoreFuncs = [];
before(() => {
restoreFuncs.push((function(value) {
if (value) {
process.env.UI_SERVER = value;
} else {
delete process.env.UI_SERVER;
}
}).bind(null, process.env.UI_SERVER));
process.env.UI_SERVER = process.env.UI_SERVER || null;
});
after(() => {
restoreFuncs.forEach(f => f());
});

it('single slash, skip', () => {
req.url = '/';

middleware.redirectOnTrailingSlash(req, res, next);
expect(next.callCount).to.equal(1);
});

it('no ending slash, skip', () => {
req.url = '/athenz';

middleware.redirectOnTrailingSlash(req, res, next);
expect(next.callCount).to.equal(1);
});

it('with ending slash, env. not set, remove and redirect', () => {
// sandbox.stub(process.env, 'UI_SERVER').value('');
sandbox.stub(process.env, 'UI_SERVER', '');
req.url = '/athenz/';

middleware.redirectOnTrailingSlash(req, res, next);
expect(res.redirect.callCount).to.equal(1);
const args = res.redirect.firstCall.args;
expect(args[0]).to.equal(301);
expect(args[1]).to.equal('//localhost/athenz');
});

it('with ending slash, env. set, remove and redirect', () => {
// sandbox.stub(process.env, 'UI_SERVER').value('athenz.server.domain');
sandbox.stub(process.env, 'UI_SERVER', 'athenz.server.domain');
req.url = '/athenz/';

middleware.redirectOnTrailingSlash(req, res, next);
expect(res.redirect.callCount).to.equal(1);
const args = res.redirect.firstCall.args;
expect(args[0]).to.equal(301);
expect(args[1]).to.equal('//athenz.server.domain/athenz');
});

it('check open redirect vulnerability', () => {
req.url = '//example.com/';

middleware.redirectOnTrailingSlash(req, res, next);
expect(res.redirect.callCount).to.equal(1);
const args = res.redirect.firstCall.args;
expect(args[0]).to.equal(301);
expect(args[1].startsWith('//example.com'), 'redirect location should not start with another domain').to.false;
});

});

});

0 comments on commit 247ed2e

Please sign in to comment.