From 6f1ab5292f80cf3aa077833070f850a2c3387bec Mon Sep 17 00:00:00 2001
From: some random guy <somerandomguy@rape.lol>
Date: Mon, 3 Aug 2020 20:19:31 +0200
Subject: [PATCH] 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`.
---
 helpers/checks/isloggedin.js | 2 +-
 models/forms/login.js        | 8 ++++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/helpers/checks/isloggedin.js b/helpers/checks/isloggedin.js
index f338b969..91880eaa 100644
--- a/helpers/checks/isloggedin.js
+++ b/helpers/checks/isloggedin.js
@@ -7,7 +7,7 @@ module.exports = async (req, res, next) => {
 	let goto;
 	if (req.method === 'GET' && req.path) {
 		//coming from a GET page isLoggedIn middleware check
-		goto = req.path;
+		goto = encodeURIComponent(req.path);
 	}
 	return res.redirect(`/login.html${goto ? '?goto='+goto : ''}`);
 }
diff --git a/models/forms/login.js b/models/forms/login.js
index b65a4211..d9d03b4d 100644
--- a/models/forms/login.js
+++ b/models/forms/login.js
@@ -8,8 +8,12 @@ module.exports = async (req, res, next) => {
 
 	const username = req.body.username.toLowerCase();
 	const password = req.body.password;
-	const goto = req.body.goto || '/account.html';
-	const failRedirect = `/login.html${goto ? '?goto='+goto : ''}`
+	let goto = req.body.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
 	const account = await Accounts.findOne(username);