Skip to content

Commit 1b32abb

Browse files
committed
fix: open redirection vulnerability
1 parent 75f297a commit 1b32abb

File tree

3 files changed

+20
-7
lines changed

3 files changed

+20
-7
lines changed

lib/controllers.js

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,13 @@ const user = require.main.require('./src/user');
1616
const meta = require.main.require('./src/meta');
1717
const helpers = require.main.require('./src/controllers/helpers');
1818

19+
const guard = (path) => {
20+
let url = new URL(path, nconf.get('url'));
21+
url = url.hostname === nconf.get('url_parsed').hostname ? url : nconf.get('url');
22+
23+
return url.toString();
24+
};
25+
1926
const Controllers = module.exports;
2027

2128

@@ -70,7 +77,7 @@ Controllers.renderTotpChallenge = async (req, res, next) => {
7077
const single = parseInt(req.query.single, 10) === 1;
7178

7279
if (req.session.tfa === true && !req.session.tfaForce) {
73-
return res.redirect(nconf.get('relative_path') + (req.query.next || '/'));
80+
return res.redirect(guard(nconf.get('relative_path') + (req.query.next || '/')));
7481
}
7582

7683
const error = req.flash('error');
@@ -97,7 +104,7 @@ Controllers.renderAuthnChallenge = async (req, res, next) => {
97104
const single = parseInt(req.query.single, 10) === 1;
98105

99106
if (req.session.tfa === true && ((req.query.next && !req.query.next.startsWith('/admin')) || !req.session.tfaForce)) {
100-
return res.redirect(nconf.get('relative_path') + (req.query.next || '/'));
107+
return res.redirect(guard(nconf.get('relative_path') + (req.query.next || '/')));
101108
}
102109

103110
if (!await parent.hasAuthn(uid)) {
@@ -137,7 +144,7 @@ Controllers.renderBackup = async (req, res, next) => {
137144
const single = parseInt(req.query.single, 10) === 1;
138145

139146
if (req.session.tfa === true && ((req.query.next && !req.query.next.startsWith('/admin')) || !req.session.tfaForce)) {
140-
return res.redirect(nconf.get('relative_path') + (req.query.next || '/'));
147+
return res.redirect(guard(nconf.get('relative_path') + (req.query.next || '/')));
141148
}
142149

143150
const error = req.flash('error');

library.js

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,12 @@ const controllerHelpers = require.main.require('./src/controllers/helpers');
2121
const SocketPlugins = require.main.require('./src/socket.io/plugins');
2222

2323
const atob = base64str => Buffer.from(base64str, 'base64').toString('binary');
24+
const guard = (path) => {
25+
let url = new URL(path, nconf.get('url'));
26+
url = url.hostname === nconf.get('url_parsed').hostname ? url : nconf.get('url');
27+
28+
return url.toString();
29+
};
2430

2531
const plugin = {
2632
_f2l: undefined,
@@ -57,15 +63,15 @@ plugin.init = async (params) => {
5763
delete req.session.tfaForce;
5864
req.session.meta.datetime = Date.now();
5965
user.auth.addSession(req.uid, req.sessionID, req.session.meta.uuid);
60-
res.redirect(nconf.get('relative_path') + (req.query.next || '/'));
66+
res.redirect(guard(nconf.get('relative_path') + (req.query.next || '/')));
6167
});
6268
hostHelpers.setupPageRoute(router, '/login/2fa/authn', [hostMiddleware.ensureLoggedIn], controllers.renderAuthnChallenge);
6369

6470
// 2fa backups codes
6571
hostHelpers.setupPageRoute(router, '/login/2fa/backup', [hostMiddleware.ensureLoggedIn], controllers.renderBackup);
6672
router.post('/login/2fa/backup', hostMiddleware.ensureLoggedIn, controllers.processBackup, (req, res) => {
6773
req.session.tfa = true;
68-
res.redirect(nconf.get('relative_path') + (req.query.next || '/'));
74+
res.redirect(guard(nconf.get('relative_path') + (req.query.next || '/')));
6975
});
7076
router.put('/login/2fa/backup', hostMiddleware.requireUser, middlewares.requireSecondFactor, hostMiddleware.applyCSRF, controllers.generateBackupCodes);
7177

@@ -167,7 +173,7 @@ plugin.addRoutes = async ({ router, middleware, helpers }) => {
167173
req.session.meta.datetime = Date.now();
168174

169175
helpers.formatApiResponse(200, res, {
170-
next: req.query.next || '/',
176+
next: guard(req.query.next || '/'),
171177
});
172178
});
173179

static/lib/authn.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ define('forum/login-authn', ['api', 'alerts', 'hooks'], function (api, alerts, h
2222
iconEl.classList.remove('fa-spin');
2323
iconEl.classList.add('fa-check');
2424
iconEl.classList.add('text-success');
25-
document.location = config.relative_path + next;
25+
document.location = next;
2626
}).catch((err) => {
2727
alerts.error(err);
2828
ajaxify.refresh();

0 commit comments

Comments
 (0)