From dbd76f0c5d77481877fba32720f6d5dc9b8187a5 Mon Sep 17 00:00:00 2001 From: webzwo0i Date: Tue, 29 Jun 2021 19:13:10 +0200 Subject: [PATCH] export: Don't leak writeable pad ID when exporting Co-authored-by: Richard Hansen --- CHANGELOG.md | 14 ++++++++++++++ src/node/handler/ExportHandler.js | 4 ++-- src/node/utils/ExportEtherpad.js | 28 ++++++++++++---------------- src/node/utils/ExportHtml.js | 4 ++-- 4 files changed, 30 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ffe8d785e..86f3a4ca5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,19 @@ # Next Release +### Security fixes + +* Fixed leak of the writable pad ID when exporting from the pad's read-only ID. + This only matters if you treat the writeable pad IDs as secret (e.g., you are + not using [ep_padlist2](https://www.npmjs.com/package/ep_padlist2)) and you + share the pad's read-only ID with untrusted users. Instead of treating + writeable pad IDs as secret, you are encouraged to take advantage of + Etherpad's authentication and authorization mechanisms (e.g., use + [ep_openid_connect](https://www.npmjs.com/package/ep_openid_connect) with + [ep_readonly_guest](https://www.npmjs.com/package/ep_readonly_guest), or write + your own + [authentication](https://etherpad.org/doc/v1.8.14/#index_authenticate) and + [authorization](https://etherpad.org/doc/v1.8.14/#index_authorize) plugins). + ### Compatibility changes * For plugin authors: diff --git a/src/node/handler/ExportHandler.js b/src/node/handler/ExportHandler.js index 0bf75a17f..f3fde047c 100644 --- a/src/node/handler/ExportHandler.js +++ b/src/node/handler/ExportHandler.js @@ -56,14 +56,14 @@ exports.doExport = async (req, res, padId, readOnlyId, type) => { // if this is a plain text export, we can do this directly // We have to over engineer this because tabs are stored as attributes and not plain text if (type === 'etherpad') { - const pad = await exportEtherpad.getPadRaw(padId); + const pad = await exportEtherpad.getPadRaw(padId, readOnlyId); res.send(pad); } else if (type === 'txt') { const txt = await exporttxt.getPadTXTDocument(padId, req.params.rev); res.send(txt); } else { // render the html document - let html = await exporthtml.getPadHTMLDocument(padId, req.params.rev); + let html = await exporthtml.getPadHTMLDocument(padId, req.params.rev, readOnlyId); // decide what to do with the html export diff --git a/src/node/utils/ExportEtherpad.js b/src/node/utils/ExportEtherpad.js index 48c850af9..45683bc65 100644 --- a/src/node/utils/ExportEtherpad.js +++ b/src/node/utils/ExportEtherpad.js @@ -19,23 +19,18 @@ const db = require('../db/DB'); const hooks = require('../../static/js/pluginfw/hooks'); -exports.getPadRaw = async (padId) => { - const padKey = `pad:${padId}`; - const padcontent = await db.get(padKey); +exports.getPadRaw = async (padId, readOnlyId) => { + const keyPrefixRead = `pad:${padId}`; + const keyPrefixWrite = readOnlyId ? `pad:${readOnlyId}` : keyPrefixRead; + const padcontent = await db.get(keyPrefixRead); - const records = [padKey]; - for (let i = 0; i <= padcontent.head; i++) { - records.push(`${padKey}:revs:${i}`); - } - - for (let i = 0; i <= padcontent.chatHead; i++) { - records.push(`${padKey}:chat:${i}`); - } + const keySuffixes = ['']; + for (let i = 0; i <= padcontent.head; i++) keySuffixes.push(`:revs:${i}`); + for (let i = 0; i <= padcontent.chatHead; i++) keySuffixes.push(`:chat:${i}`); const data = {}; - for (const key of records) { - // For each piece of info about a pad. - const entry = data[key] = await db.get(key); + for (const keySuffix of keySuffixes) { + const entry = data[keyPrefixWrite + keySuffix] = await db.get(keyPrefixRead + keySuffix); // Get the Pad Authors if (entry.pool && entry.pool.numToAttrib) { @@ -50,7 +45,7 @@ exports.getPadRaw = async (padId) => { if (authorEntry) { data[`globalAuthor:${authorId}`] = authorEntry; if (authorEntry.padIDs) { - authorEntry.padIDs = padId; + authorEntry.padIDs = readOnlyId || padId; } } } @@ -63,7 +58,8 @@ exports.getPadRaw = async (padId) => { const prefixes = await hooks.aCallAll('exportEtherpadAdditionalContent'); await Promise.all(prefixes.map(async (prefix) => { const key = `${prefix}:${padId}`; - data[key] = await db.get(key); + const writeKey = readOnlyId ? `${prefix}:${readOnlyId}` : key; + data[writeKey] = await db.get(key); })); return data; diff --git a/src/node/utils/ExportHtml.js b/src/node/utils/ExportHtml.js index 38e5fb1a6..bc50da77b 100644 --- a/src/node/utils/ExportHtml.js +++ b/src/node/utils/ExportHtml.js @@ -457,7 +457,7 @@ const getHTMLFromAtext = async (pad, atext, authorColors) => { return pieces.join(''); }; -exports.getPadHTMLDocument = async (padId, revNum) => { +exports.getPadHTMLDocument = async (padId, revNum, readOnlyId) => { const pad = await padManager.getPad(padId); // Include some Styles into the Head for Export @@ -475,7 +475,7 @@ exports.getPadHTMLDocument = async (padId, revNum) => { return eejs.require('ep_etherpad-lite/templates/export_html.html', { body: html, - padId: Security.escapeHTML(padId), + padId: Security.escapeHTML(readOnlyId || padId), extraCSS: stylesForExportCSS, }); };