diff --git a/app/controllers/api/login.js b/app/controllers/api/login.js index 8b08499c9..cda8c281f 100644 --- a/app/controllers/api/login.js +++ b/app/controllers/api/login.js @@ -1,6 +1,8 @@ /** * login controller, to authenticate users */ +const snip = require('../../snip.js'); +const config = require('../../models/config.js'); var emailModel = require('../../models/email.js'); var userModel = require('../../models/user.js'); var notifEmails = require('../../models/notifEmails.js'); @@ -60,7 +62,12 @@ exports.handleRequest = function (request, form, response, ignorePassword) { userModel[form.email.indexOf('@') > -1 ? 'fetchByEmail' : 'fetchByHandle']( form.email, function (dbUser) { - if (!dbUser) { + if ( + form.redirect && + snip.getSafeOpenwhydURL(form.redirect, config.urlPrefix) === false + ) { + form.error = 'Unsafe redirect target'; + } else if (!dbUser) { form.error = "Are you sure? We don't recognize your email address!"; } else if (form.action == 'forgot') { notifEmails.sendPasswordReset(dbUser._id, dbUser.pwd, form.redirect); diff --git a/app/models/logging.js b/app/models/logging.js index 472457690..d0f9e49a0 100644 --- a/app/models/logging.js +++ b/app/models/logging.js @@ -261,9 +261,8 @@ http.ServerResponse.prototype.redirect = function (url) { }; http.ServerResponse.prototype.safeRedirect = function (url) { - const fullURL = new URL(url, config.urlPrefix); - if (`${fullURL.protocol}//${fullURL.host}` !== config.urlPrefix) - return this.forbidden(); + const safeURL = snip.getSafeOpenwhydURL(url, config.urlPrefix); + if (safeURL === false) return this.forbidden(); this.redirect(url); }; diff --git a/app/snip.js b/app/snip.js index 753aaad9b..0b8955f88 100644 --- a/app/snip.js +++ b/app/snip.js @@ -109,6 +109,13 @@ exports.sanitizeJsStringInHtml = function (str) { return exports.htmlEntities(exports.addSlashes(str || '')); }; +exports.getSafeOpenwhydURL = function (url, safeUrlPrefix) { + if (typeof url !== 'string' || url.includes(' { - describe('Open Redirect', () => { + describe('Open Redirect from /login', () => { + it('should allow redirect to /stream', async () => { + const target = `/stream`; + const { response } = await postRaw(null, `/login`, { + action: 'login', + email: ADMIN_USER.email, + md5: ADMIN_USER.md5, + redirect: target, + }); + assert( + response.body.includes(`window.location.href="${target}"`) === true, + `page body should include redirect to ${target}` + ); + }); + + it('should NOT allow redirect to other domain', async () => { + const target = `https://google.com`; + const { response } = await postRaw(null, `/login`, { + action: 'login', + email: ADMIN_USER.email, + md5: ADMIN_USER.md5, + redirect: target, + }); + assert( + response.body.includes(`window.location.href="${target}"`) === false, + `page body should NOT include redirect to ${target}` + ); + }); + + it('should NOT allow javascript in redirect URL', async () => { + const target = `javascript:alert()`; + const { response } = await postRaw(null, `/login`, { + action: 'login', + email: ADMIN_USER.email, + md5: ADMIN_USER.md5, + redirect: target, + }); + assert( + response.body.includes(`window.location.href="${target}"`) === false, + `page body should NOT include redirect to ${target}` + ); + }); + + it('should NOT allow script element in redirect URL', async () => { + const target = ``; + const { response } = await postRaw(null, `/login`, { + action: 'login', + email: ADMIN_USER.email, + md5: ADMIN_USER.md5, + redirect: target, + }); + assert( + response.body.includes(`window.location.href="${target}"`) === false, + `page body should NOT include redirect to ${target}` + ); + }); + }); + + describe('Open Redirect from /consent', () => { it('should allow redirect to /stream', async () => { const target = `/stream`; const { jar } = await loginAs(ADMIN_USER);