safer redirects with login/logout

* properly escape goto parameter
* do not redirect to anywhere, only to the same server, no query parameters

This should still allow valid targets, like `/account.html`,
`/boardname/manage/whatever` while disallow things like `https://othersite.com`.
merge-requests/208/head
some random guy 4 years ago
parent d6567bdbbe
commit 6f1ab5292f
  1. 2
      helpers/checks/isloggedin.js
  2. 8
      models/forms/login.js

@ -7,7 +7,7 @@ module.exports = async (req, res, next) => {
let goto; let goto;
if (req.method === 'GET' && req.path) { if (req.method === 'GET' && req.path) {
//coming from a GET page isLoggedIn middleware check //coming from a GET page isLoggedIn middleware check
goto = req.path; goto = encodeURIComponent(req.path);
} }
return res.redirect(`/login.html${goto ? '?goto='+goto : ''}`); return res.redirect(`/login.html${goto ? '?goto='+goto : ''}`);
} }

@ -8,8 +8,12 @@ module.exports = async (req, res, next) => {
const username = req.body.username.toLowerCase(); const username = req.body.username.toLowerCase();
const password = req.body.password; const password = req.body.password;
const goto = req.body.goto || '/account.html'; let goto = req.body.goto;
const failRedirect = `/login.html${goto ? '?goto='+goto : ''}` // we don't want to redirect the user to random sites
if (goto == null || !/^\/[0-9a-zA-Z][0-9a-zA-Z._/-]*$/.test(goto)) {
goto = '/account.html';
}
const failRedirect = `/login.html${goto ? '?goto='+encodeURIComponent(goto) : ''}`
//fetch an account //fetch an account
const account = await Accounts.findOne(username); const account = await Accounts.findOne(username);

Loading…
Cancel
Save