From 6134c61c0a8e759b39384411f8db31d9e5a72891 Mon Sep 17 00:00:00 2001 From: John McLear Date: Sat, 25 Jul 2026 14:35:37 +0100 Subject: [PATCH 1/2] security(export): strip remote images on the soffice path to match the native path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Defense-in-depth / consistency fix — not a core vulnerability on its own. Core Etherpad never emits tags in export HTML; they only appear when a plugin/hook injects them, so sanitising such content is primarily the injecting plugin's responsibility. However, the native in-process export path (issue #7538) already calls stripRemoteImages() defensively, while the LibreOffice (soffice) path wrote the HTML to the temp file verbatim. soffice is the only export path that actually performs outbound fetches for remote URLs during conversion, so a plugin-injected remote image there becomes a blind SSRF sink. This makes the soffice path consistent with the native path core already ships. Adds a regression test that injects a remote image via exportHTMLAdditionalContent, intercepts the temp file via the exportConvert hook, and asserts no remote image URL survives. Remote-image behaviour reported privately by meifukun. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/node/handler/ExportHandler.ts | 8 +- .../backend/specs/sofficeExportRemoteImage.ts | 108 ++++++++++++++++++ 2 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 src/tests/backend/specs/sofficeExportRemoteImage.ts diff --git a/src/node/handler/ExportHandler.ts b/src/node/handler/ExportHandler.ts index 655cf6497ae..d141aba676a 100644 --- a/src/node/handler/ExportHandler.ts +++ b/src/node/handler/ExportHandler.ts @@ -163,7 +163,13 @@ exports.doExport = async (req: any, res: any, padId: string, readOnlyId: string, // for the temp path token (see matching note in ImportHandler.ts). const randNum = crypto.randomBytes(16).toString('hex'); const srcFile = `${tempDirectory}/etherpad_export_${randNum}.html`; - await fsp_writeFile(srcFile, html); + // Strip remote tags before handing the document to LibreOffice. + // soffice fetches remote image URLs during conversion, so any plugin/hook + // that injects an into export HTML would otherwise + // turn export into a blind SSRF sink. The native path already does this + // (see stripRemoteImages above); apply it here so both paths match. + const {stripRemoteImages} = require('../utils/ExportSanitizeHtml'); + await fsp_writeFile(srcFile, stripRemoteImages(html)); // ensure html can be collected by the garbage collector html = null; diff --git a/src/tests/backend/specs/sofficeExportRemoteImage.ts b/src/tests/backend/specs/sofficeExportRemoteImage.ts new file mode 100644 index 00000000000..270f926b5c5 --- /dev/null +++ b/src/tests/backend/specs/sofficeExportRemoteImage.ts @@ -0,0 +1,108 @@ +'use strict'; + +/** + * SSRF regression for the LibreOffice (`soffice`) export path. + * + * The native (in-process) export path strips remote `` tags before + * rendering, but the `soffice` path historically wrote the full export HTML to + * disk verbatim and handed it to LibreOffice, which fetches remote image URLs + * during conversion — a blind SSRF sink reachable whenever a plugin/hook injects + * image tags into export HTML. Reported by `meifukun`. + * + * This test drives the real export handler with: + * - an `exportHTMLAdditionalContent` hook that injects a remote image (the + * documented plugin extension point from the PoC), and + * - an `exportConvert` hook that stands in for LibreOffice: it captures the + * exact HTML written to the temp file and writes a dummy output so the + * request completes. + * It then asserts the captured HTML contains no remote image URL. + */ + +import {MapArrayType} from "../../../node/types/MapType"; +import fs from 'node:fs'; +const fsp = fs.promises; + +const assert = require('assert').strict; +const common = require('../common'); +const padManager = require('../../../node/db/PadManager'); +const plugins = require('../../../static/js/pluginfw/plugin_defs'); +import settings from '../../../node/utils/Settings'; + +const REMOTE_IMG = 'probe'; + +describe(__filename, function () { + this.timeout(30000); + let agent: any; + const settingsBackup: MapArrayType = {}; + const hookBackup: MapArrayType = {}; + const hookNames = ['exportHTMLAdditionalContent', 'exportConvert']; + let capturedHtml = ''; + + const makeHook = (hookName: string, hookFn: Function) => ({ + hook_fn: hookFn, + hook_fn_name: `ssrf_test/${hookName}`, + hook_name: hookName, + part: {plugin: 'ssrf_test'}, + }); + + before(async function () { + agent = await common.init(); + settingsBackup.soffice = settings.soffice; + await padManager.getPad('sofficeSsrfPad', 'test content'); + }); + + beforeEach(function () { + for (const h of hookNames) { + hookBackup[h] = plugins.hooks[h]; + } + capturedHtml = ''; + // Inject a remote image the way a plugin extension point would. + plugins.hooks.exportHTMLAdditionalContent = [ + makeHook('exportHTMLAdditionalContent', () => REMOTE_IMG), + ]; + // Stand in for LibreOffice: capture what was written, produce a dummy file. + plugins.hooks.exportConvert = [ + makeHook('exportConvert', async (hookName: string, {srcFile, destFile}: any) => { + capturedHtml = await fsp.readFile(srcFile, 'utf8'); + await fsp.writeFile(destFile, 'dummy-converted-output'); + return 'handled'; + }), + ]; + // Force the soffice conversion path (non-null => sofficeAvailable() === 'yes' + // on non-Windows). The binary is never actually spawned because the + // exportConvert hook short-circuits the converter. + settings.soffice = '/usr/bin/soffice'; + }); + + afterEach(function () { + for (const h of hookNames) { + plugins.hooks[h] = hookBackup[h]; + } + }); + + after(function () { + Object.assign(settings, settingsBackup); + }); + + it('confirms the injected remote image reaches the export HTML', async function () { + // Sanity check on the harness: the raw HTML export (which is NOT run through + // the soffice temp-file path) must contain the injected image, proving the + // hook fires and the marker would otherwise be present. + const res = await agent.get('/p/sofficeSsrfPad/export/html').expect(200); + assert.ok( + res.text.includes('ssrf-marker-soffice'), + 'expected the injected remote image to appear in the html export'); + }); + + it('strips remote images from the HTML handed to the soffice converter', + async function () { + await agent.get('/p/sofficeSsrfPad/export/odt').expect(200); + assert.ok(capturedHtml.length > 0, 'exportConvert hook did not capture any HTML'); + assert.ok( + !capturedHtml.includes('ssrf-marker-soffice'), + `soffice temp file still contains the remote image URL:\n${capturedHtml}`); + assert.ok( + !/]+src\s*=\s*["']?https?:/i.test(capturedHtml), + `soffice temp file still contains a remote :\n${capturedHtml}`); + }); +}); From 5a69a8aec4bf611fd97bbe0d37a0ce27243ba3e6 Mon Sep 17 00:00:00 2001 From: John McLear Date: Sat, 25 Jul 2026 16:39:28 +0100 Subject: [PATCH 2/2] export: preserve doctype and comments in stripRemoteImages Addresses Qodo review on #8071. Applying stripRemoteImages() to the full export document for the soffice path dropped `` and HTML comments, because the htmlparser2 handler only emitted open/text/close tags. A missing doctype can flip LibreOffice's HTML import into quirks mode, subtly changing rendering for all soffice exports. Add onprocessinginstruction (doctype) and oncomment handlers so the serializer round-trips document directives and comments while still stripping remote images. The native path is unaffected (it strips only extractBody() output, which has no doctype). Adds regression tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/node/utils/ExportSanitizeHtml.ts | 11 +++++++++++ src/tests/backend/specs/export.ts | 20 ++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/src/node/utils/ExportSanitizeHtml.ts b/src/node/utils/ExportSanitizeHtml.ts index bc2028bcd60..91e287c88f7 100644 --- a/src/node/utils/ExportSanitizeHtml.ts +++ b/src/node/utils/ExportSanitizeHtml.ts @@ -208,6 +208,17 @@ export const stripRemoteImages = (html: string): string => { if (VOID_TAGS.has(name)) return; out += ``; }, + // Preserve document-level directives (notably ``) and + // comments. stripRemoteImages() runs on the FULL export document for the + // soffice path, so dropping the doctype would flip LibreOffice into quirks + // mode. htmlparser2 surfaces the doctype as a processing instruction whose + // `data` is e.g. `!doctype html`. + onprocessinginstruction(name, data) { + out += `<${data}>`; + }, + oncomment(data) { + out += ``; + }, }, {decodeEntities: false, lowerCaseTags: true}); parser.write(html); parser.end(); diff --git a/src/tests/backend/specs/export.ts b/src/tests/backend/specs/export.ts index b3fb94c673e..0a78d27e06d 100644 --- a/src/tests/backend/specs/export.ts +++ b/src/tests/backend/specs/export.ts @@ -150,6 +150,26 @@ describe(__filename, function () { const html = '

hi

body link

'; assert.strictEqual(stripRemoteImages(html), html); }); + + it('preserves the doctype directive (soffice reads a full document)', function () { + const html = '

hi

'; + const out = stripRemoteImages(html); + assert.match(out, //i); + }); + + it('preserves HTML comments', function () { + const html = '

hi

'; + const out = stripRemoteImages(html); + assert.match(out, //); + }); + + it('preserves the doctype while still dropping a remote image', function () { + const html = + 'a'; + const out = stripRemoteImages(html); + assert.match(out, //i); + assert.doesNotMatch(out, /evil\.example/); + }); }); describe('extractBody', function () {