mirror of
https://github.com/ether/etherpad-lite.git
synced 2026-07-29 13:00:27 +00:00
security(export): strip remote images on the soffice path to match the native path (#8071)
* security(export): strip remote images on the soffice path to match the native path Defense-in-depth / consistency fix — not a core vulnerability on its own. Core Etherpad never emits <img> 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 <img> 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) <noreply@anthropic.com> * export: preserve doctype and comments in stripRemoteImages Addresses Qodo review on #8071. Applying stripRemoteImages() to the full export document for the soffice path dropped `<!doctype html>` 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
John McLear
GitHub
parent
f3ccc6c718
commit
492b158d93
4 changed files with 146 additions and 1 deletions
|
|
@ -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 <img> tags before handing the document to LibreOffice.
|
||||
// soffice fetches remote image URLs during conversion, so any plugin/hook
|
||||
// that injects an <img src="http://..."> 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;
|
||||
|
|
|
|||
|
|
@ -208,6 +208,17 @@ export const stripRemoteImages = (html: string): string => {
|
|||
if (VOID_TAGS.has(name)) return;
|
||||
out += `</${name}>`;
|
||||
},
|
||||
// Preserve document-level directives (notably `<!doctype html>`) 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 += `<!--${data}-->`;
|
||||
},
|
||||
}, {decodeEntities: false, lowerCaseTags: true});
|
||||
parser.write(html);
|
||||
parser.end();
|
||||
|
|
|
|||
|
|
@ -150,6 +150,26 @@ describe(__filename, function () {
|
|||
const html = '<h1>hi</h1><p>body <a href="/x">link</a></p>';
|
||||
assert.strictEqual(stripRemoteImages(html), html);
|
||||
});
|
||||
|
||||
it('preserves the doctype directive (soffice reads a full document)', function () {
|
||||
const html = '<!doctype html><html><body><p>hi</p></body></html>';
|
||||
const out = stripRemoteImages(html);
|
||||
assert.match(out, /<!doctype html>/i);
|
||||
});
|
||||
|
||||
it('preserves HTML comments', function () {
|
||||
const html = '<body><!-- keep me --><p>hi</p></body>';
|
||||
const out = stripRemoteImages(html);
|
||||
assert.match(out, /<!-- keep me -->/);
|
||||
});
|
||||
|
||||
it('preserves the doctype while still dropping a remote image', function () {
|
||||
const html =
|
||||
'<!doctype html><html><body><img src="https://evil.example/x.png" alt="a"></body></html>';
|
||||
const out = stripRemoteImages(html);
|
||||
assert.match(out, /<!doctype html>/i);
|
||||
assert.doesNotMatch(out, /evil\.example/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('extractBody', function () {
|
||||
|
|
|
|||
108
src/tests/backend/specs/sofficeExportRemoteImage.ts
Normal file
108
src/tests/backend/specs/sofficeExportRemoteImage.ts
Normal file
|
|
@ -0,0 +1,108 @@
|
|||
'use strict';
|
||||
|
||||
/**
|
||||
* SSRF regression for the LibreOffice (`soffice`) export path.
|
||||
*
|
||||
* The native (in-process) export path strips remote `<img>` 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 = '<img src="http://127.0.0.1:39111/ssrf-marker-soffice" alt="probe">';
|
||||
|
||||
describe(__filename, function () {
|
||||
this.timeout(30000);
|
||||
let agent: any;
|
||||
const settingsBackup: MapArrayType<any> = {};
|
||||
const hookBackup: MapArrayType<any> = {};
|
||||
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(
|
||||
!/<img[^>]+src\s*=\s*["']?https?:/i.test(capturedHtml),
|
||||
`soffice temp file still contains a remote <img src="http...">:\n${capturedHtml}`);
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue