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/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 () { 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}`); + }); +});