mirror of
https://github.com/ether/etherpad-lite.git
synced 2025-04-22 00:16:15 -04:00
security: Fix authorization bypass vulnerability
Before, a malicious user could bypass authorization restrictions imposed by the authorize hook: * Step 1: Fetch any resource that the malicious user is authorized to access (e.g., static content). * Step 2: Use the signed express_sid cookie generated in step 1 to create a socket.io connection. * Step 3: Perform the CLIENT_READY handshake for the desired pad. * Step 4: Profit! Now the authorization decision made by the authorize hook is propagated to SecurityManager so that it can approve or reject socket.io messages as appropriate. This also sets up future support for per-user read-only and modify-only (no create) authorization levels.
This commit is contained in:
parent
ae1142a799
commit
b80a37173e
4 changed files with 63 additions and 25 deletions
|
@ -23,6 +23,7 @@ var hooks = require("ep_etherpad-lite/static/js/pluginfw/hooks.js");
|
|||
var padManager = require("./PadManager");
|
||||
var sessionManager = require("./SessionManager");
|
||||
var settings = require("../utils/Settings");
|
||||
const webaccess = require('../hooks/express/webaccess');
|
||||
var log4js = require('log4js');
|
||||
var authLogger = log4js.getLogger("auth");
|
||||
|
||||
|
@ -57,11 +58,21 @@ exports.checkAccess = async function(padID, sessionCookie, token, password, user
|
|||
return DENY;
|
||||
}
|
||||
|
||||
// Make sure the user has authenticated if authentication is required. The caller should have
|
||||
// already performed this check, but it is repeated here just in case.
|
||||
if (settings.requireAuthentication && userSettings == null) {
|
||||
authLogger.debug('access denied: authentication is required');
|
||||
return DENY;
|
||||
if (settings.requireAuthentication) {
|
||||
// Make sure the user has authenticated if authentication is required. The caller should have
|
||||
// already performed this check, but it is repeated here just in case.
|
||||
if (userSettings == null) {
|
||||
authLogger.debug('access denied: authentication is required');
|
||||
return DENY;
|
||||
}
|
||||
// Check whether the user is authorized. Note that userSettings.padAuthorizations will still be
|
||||
// populated even if settings.requireAuthorization is false.
|
||||
const padAuthzs = userSettings.padAuthorizations || {};
|
||||
const level = webaccess.normalizeAuthzLevel(padAuthzs[padID]);
|
||||
if (!level) {
|
||||
authLogger.debug('access denied: unauthorized');
|
||||
return DENY;
|
||||
}
|
||||
}
|
||||
|
||||
// allow plugins to deny access
|
||||
|
|
|
@ -8,6 +8,19 @@ const stats = require('ep_etherpad-lite/node/stats');
|
|||
const sessionModule = require('express-session');
|
||||
const cookieParser = require('cookie-parser');
|
||||
|
||||
exports.normalizeAuthzLevel = (level) => {
|
||||
if (!level) return false;
|
||||
switch (level) {
|
||||
case true:
|
||||
return 'create';
|
||||
case 'create':
|
||||
return level;
|
||||
default:
|
||||
httpLogger.warn(`Unknown authorization level '${level}', denying access`);
|
||||
}
|
||||
return false;
|
||||
};
|
||||
|
||||
exports.checkAccess = (req, res, next) => {
|
||||
const hookResultMangle = (cb) => {
|
||||
return (err, data) => {
|
||||
|
@ -21,17 +34,28 @@ exports.checkAccess = (req, res, next) => {
|
|||
// Do not require auth for static paths and the API...this could be a bit brittle
|
||||
if (req.path.match(/^\/(static|javascripts|pluginfw|api)/)) return next();
|
||||
|
||||
const grant = (level) => {
|
||||
level = exports.normalizeAuthzLevel(level);
|
||||
if (!level) return fail();
|
||||
const user = req.session.user;
|
||||
if (user == null) return next(); // This will happen if authentication is not required.
|
||||
const padID = (req.path.match(/^\/p\/(.*)$/) || [])[1];
|
||||
if (padID == null) return next();
|
||||
// The user was granted access to a pad. Remember the authorization level in the user's
|
||||
// settings so that SecurityManager can approve or deny specific actions.
|
||||
if (user.padAuthorizations == null) user.padAuthorizations = {};
|
||||
user.padAuthorizations[padID] = level;
|
||||
return next();
|
||||
};
|
||||
|
||||
if (req.path.toLowerCase().indexOf('/admin') !== 0) {
|
||||
if (!settings.requireAuthentication) return next();
|
||||
if (!settings.requireAuthorization && req.session && req.session.user) return next();
|
||||
if (!settings.requireAuthentication) return grant('create');
|
||||
if (!settings.requireAuthorization && req.session && req.session.user) return grant('create');
|
||||
}
|
||||
|
||||
if (req.session && req.session.user && req.session.user.is_admin) return next();
|
||||
if (req.session && req.session.user && req.session.user.is_admin) return grant('create');
|
||||
|
||||
hooks.aCallFirst('authorize', {req, res, next, resource: req.path}, hookResultMangle((ok) => {
|
||||
if (ok) return next();
|
||||
return fail();
|
||||
}));
|
||||
hooks.aCallFirst('authorize', {req, res, next, resource: req.path}, hookResultMangle(grant));
|
||||
};
|
||||
|
||||
/* Authentication OR authorization failed. */
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue