From 60ea7d51212f0fbdfca5bd583d47271f7fd746fd Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Mon, 22 Aug 2016 16:24:28 -0400 Subject: [PATCH] fixes #4966 --- app.js | 2 ++ public/src/ajaxify.js | 4 +--- public/src/utils.js | 8 ++++++++ src/controllers/authentication.js | 11 ++++++++--- 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/app.js b/app.js index 2158d61282..74beb77d6e 100644 --- a/app.js +++ b/app.js @@ -103,6 +103,8 @@ function loadConfig() { nconf.set('themes_path', path.resolve(__dirname, nconf.get('themes_path'))); nconf.set('core_templates_path', path.join(__dirname, 'src/views')); nconf.set('base_templates_path', path.join(nconf.get('themes_path'), 'nodebb-theme-persona/templates')); + + nconf.set('url_parsed', url.parse(nconf.get('url'))); } diff --git a/public/src/ajaxify.js b/public/src/ajaxify.js index de20693eb0..d0592d8511 100644 --- a/public/src/ajaxify.js +++ b/public/src/ajaxify.js @@ -334,9 +334,7 @@ $(document).ready(function() { return; } - var internalLink = this.host === '' || // Relative paths are always internal links - (this.host === window.location.host && this.protocol === window.location.protocol && // Otherwise need to check if protocol and host match - (RELATIVE_PATH.length > 0 ? this.pathname.indexOf(RELATIVE_PATH) === 0 : true)); // Subfolder installs need this additional check + var internalLink = utils.isInternalURI(this, window.location, RELATIVE_PATH); if ($(this).attr('data-ajaxify') === 'false') { if (!internalLink) { diff --git a/public/src/utils.js b/public/src/utils.js index 5d44bcd4fa..190d0ab3c6 100644 --- a/public/src/utils.js +++ b/public/src/utils.js @@ -431,6 +431,14 @@ } return utils.props(obj[prop], newProps, value); + }, + + isInternalURI: function(targetLocation, referenceLocation, relative_path) { + return targetLocation.host === '' || // Relative paths are always internal links + ( + targetLocation.host === referenceLocation.host && targetLocation.protocol === referenceLocation.protocol && // Otherwise need to check if protocol and host match + (relative_path.length > 0 ? targetLocation.pathname.indexOf(relative_path) === 0 : true) // Subfolder installs need this additional check + ); } }; diff --git a/src/controllers/authentication.js b/src/controllers/authentication.js index 98dc6c1fe4..0f74827be2 100644 --- a/src/controllers/authentication.js +++ b/src/controllers/authentication.js @@ -6,6 +6,7 @@ var passport = require('passport'); var nconf = require('nconf'); var validator = require('validator'); var _ = require('underscore'); +var url = require('url'); var db = require('../database'); var meta = require('../meta'); @@ -168,7 +169,7 @@ authenticationController.registerComplete = function(req, res, next) { } else { res.redirect(nconf.get('relative_path') + '/'); } - } + }; async.parallel(callbacks, function(err) { if (err) { @@ -187,7 +188,7 @@ authenticationController.registerComplete = function(req, res, next) { }); }; -authenticationController.registerAbort = function(req, res, next) { +authenticationController.registerAbort = function(req, res) { // End the session and redirect to home req.session.destroy(function() { res.redirect(nconf.get('relative_path') + '/'); @@ -197,7 +198,11 @@ authenticationController.registerAbort = function(req, res, next) { authenticationController.login = function(req, res, next) { // Handle returnTo data if (req.body.hasOwnProperty('returnTo') && !req.session.returnTo) { - req.session.returnTo = req.body.returnTo; + // As req.body is data obtained via userland, it is untrusted, restrict to internal links only + var parsed = url.parse(req.body.returnTo); + var isInternal = utils.isInternalURI(url.parse(req.body.returnTo), nconf.get('url_parsed'), nconf.get('relative_path')); + + req.session.returnTo = isInternal ? req.body.returnTo : nconf.get('url'); } if (plugins.hasListeners('action:auth.overrideLogin')) {