From 875337d9a659692072d488bf54c207dd0614764a Mon Sep 17 00:00:00 2001 From: Calvin Montgomery Date: Sun, 5 Nov 2017 16:17:37 -0800 Subject: [PATCH] web/account: add referrer check --- NEWS.md | 13 ++++++++++++ package.json | 2 +- src/web/account.js | 49 ++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 61 insertions(+), 3 deletions(-) diff --git a/NEWS.md b/NEWS.md index 039c99b3..cb95323c 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,16 @@ +2017-11-05 +========== + +The latest commit introduces a referrer check in the account page handlers. +This is added as a short-term mitigation for a recent report that account +management functions (such as deleting channels) can be executed without the +user's consent if placed in channel JS. + +Longer term options are being considered, such as moving account management to a +separate subdomain to take advantage of cross-origin checks in browsers, and +requiring the user to re-enter their password to demonstrate intent. As always, +I recommend admins take extreme caution when accepting channel JS. + 2017-09-26 ========== diff --git a/package.json b/package.json index 6b161a2a..7fdf8f35 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "author": "Calvin Montgomery", "name": "CyTube", "description": "Online media synchronizer and chat", - "version": "3.51.2", + "version": "3.51.3", "repository": { "url": "http://github.com/calzoneman/sync" }, diff --git a/src/web/account.js b/src/web/account.js index fb241123..510929b0 100644 --- a/src/web/account.js +++ b/src/web/account.js @@ -15,7 +15,7 @@ var session = require("../session"); var csrf = require("./csrf"); const url = require("url"); -const LOGGER = require('@calzoneman/jsli')('database/accounts'); +const LOGGER = require('@calzoneman/jsli')('web/accounts'); let globalMessageBus; let emailConfig; @@ -28,12 +28,42 @@ function handleAccountEditPage(req, res) { sendPug(res, "account-edit", {}); } +function verifyReferrer(req, expected) { + const referrer = req.header('referer'); + + if (!referrer) { + return true; + } + + try { + const parsed = url.parse(referrer); + + if (parsed.pathname !== expected) { + LOGGER.warn( + 'Possible attempted forgery: %s POSTed to %s', + referrer, + expected + ); + return false; + } + + return true; + } catch (error) { + return false; + } +} + /** * Handles a POST request to edit a user"s account */ function handleAccountEdit(req, res) { csrf.verify(req); + if (!verifyReferrer(req, '/account/edit')) { + res.status(403).send('Mismatched referrer'); + return; + } + var action = req.body.action; switch(action) { case "change_password": @@ -43,7 +73,7 @@ function handleAccountEdit(req, res) { handleChangeEmail(req, res); break; default: - res.send(400); + res.sendStatus(400); break; } } @@ -197,6 +227,11 @@ async function handleAccountChannelPage(req, res) { function handleAccountChannel(req, res) { csrf.verify(req); + if (!verifyReferrer(req, '/account/channels')) { + res.status(403).send('Mismatched referrer'); + return; + } + var action = req.body.action; switch(action) { case "new_channel": @@ -395,6 +430,11 @@ function validateProfileImage(image, callback) { async function handleAccountProfile(req, res) { csrf.verify(req); + if (!verifyReferrer(req, '/account/profile')) { + res.status(403).send('Mismatched referrer'); + return; + } + const user = await webserver.authorize(req); // TODO: error message if (!user) { @@ -465,6 +505,11 @@ function handlePasswordResetPage(req, res) { function handlePasswordReset(req, res) { csrf.verify(req); + if (!verifyReferrer(req, '/account/passwordreset')) { + res.status(403).send('Mismatched referrer'); + return; + } + var name = req.body.name, email = req.body.email;