From b878a46c4c2f6c8371e3ec47966464866ddd3e3b Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Sat, 22 Aug 2026 16:49:48 -0700 Subject: [PATCH] fix(charts): confine untrusted chart options to canvas render paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `.chart` document is untrusted input — any workspace member authors one, and a public share link renders it to anonymous visitors on the app origin — but `parseChartSpec` forwarded `option` to `setOption` unchanged. ECharts draws through canvas with two exceptions: a `tooltip` left in its default `renderMode: 'html'` assigns its content to `innerHTML`, and a string `formatter` is that content's template verbatim (only substituted values are escaped), while `toolbox` assigns `dataView.lang` to `innerHTML` and fills a `saveAsImage` popup with `document.write`. Force the render mode and drop the toolbox so the document has no DOM sink, rather than filtering the values that flow through one. The walk is deep: `baseOption`, `media[].option` and timeline `options[]` each carry their own tooltip, a tooltip declared only under `media` still instantiates the component in HTML mode, and a `media` entry can override a top-level `renderMode`. Also escape the series name in the pptx chart renderer's bubble tooltip — it comes from the uploaded document and is interpolated into a hand-built `innerHTML` string, where ECharts escapes only the markup it builds itself. --- apps/sim/lib/charts/spec.test.ts | 185 ++++++++++++++++++ apps/sim/lib/charts/spec.ts | 66 ++++++- .../pptx-renderer/renderer/chart-renderer.ts | 5 +- 3 files changed, 251 insertions(+), 5 deletions(-) diff --git a/apps/sim/lib/charts/spec.test.ts b/apps/sim/lib/charts/spec.test.ts index da150499d49..c0bf9cd2046 100644 --- a/apps/sim/lib/charts/spec.test.ts +++ b/apps/sim/lib/charts/spec.test.ts @@ -1,9 +1,18 @@ /** * @vitest-environment node */ +import * as echarts from 'echarts' import { describe, expect, it } from 'vitest' +import { buildChartRenderOption } from '@/lib/charts/option' import { parseChartSpec, shapeTableRows } from '@/lib/charts/spec' +/** Parses a chart document written as a plain object, asserting it was accepted. */ +function parse(doc: Record): Record { + const { spec, error } = parseChartSpec(JSON.stringify(doc)) + expect(error).toBeUndefined() + return spec!.option +} + const rows = [ { month: '2024-01', region: 'NA', revenue: 100, conversion: 4 }, { month: '2024-01', region: 'EMEA', revenue: 50, conversion: 2 }, @@ -85,6 +94,182 @@ describe('shapeTableRows', () => { }) }) +const XSS_FORMATTER = '' + +describe('parseChartSpec option confinement', () => { + it('forces the tooltip off the innerHTML path, keeping the formatter template', () => { + const option = parse({ + schema_version: 1, + option: { tooltip: { trigger: 'item', formatter: XSS_FORMATTER }, series: [{ type: 'bar' }] }, + }) + expect(option.tooltip).toEqual({ + trigger: 'item', + formatter: XSS_FORMATTER, + renderMode: 'richText', + }) + }) + + it('overrides a spec-declared html render mode', () => { + const option = parse({ + schema_version: 1, + option: { tooltip: { renderMode: 'html', formatter: XSS_FORMATTER } }, + }) + expect((option.tooltip as Record).renderMode).toBe('richText') + }) + + it('reaches tooltips nested under media, baseOption, timeline options, and series', () => { + const option = parse({ + schema_version: 1, + option: { + baseOption: { tooltip: { formatter: XSS_FORMATTER } }, + options: [{ tooltip: { formatter: XSS_FORMATTER } }], + media: [{ query: { minWidth: 100 }, option: { tooltip: { formatter: XSS_FORMATTER } } }], + series: [ + { + type: 'bar', + tooltip: { formatter: XSS_FORMATTER }, + data: [{ value: 1, tooltip: { formatter: XSS_FORMATTER } }], + }, + ], + }, + }) + const renderModes: unknown[] = [] + function collect(node: unknown): void { + if (node === null || typeof node !== 'object') return + if (Array.isArray(node)) { + for (const entry of node) collect(entry) + return + } + const record = node as Record + if (record.formatter === XSS_FORMATTER) renderModes.push(record.renderMode) + for (const value of Object.values(record)) collect(value) + } + collect(option) + expect(renderModes).toHaveLength(5) + expect(renderModes.every((mode) => mode === 'richText')).toBe(true) + }) + + it('confines a tooltip declared as an array', () => { + const option = parse({ + schema_version: 1, + option: { tooltip: [{ formatter: XSS_FORMATTER }, { formatter: 'plain' }] }, + }) + expect(option.tooltip).toEqual([ + { formatter: XSS_FORMATTER, renderMode: 'richText' }, + { formatter: 'plain', renderMode: 'richText' }, + ]) + }) + + it('drops the toolbox at every level', () => { + const option = parse({ + schema_version: 1, + option: { + toolbox: { feature: { dataView: { lang: [XSS_FORMATTER] } } }, + baseOption: { toolbox: { feature: { saveAsImage: {} } } }, + media: [{ query: { minWidth: 100 }, option: { toolbox: { show: true } } }], + }, + }) + expect(option.toolbox).toBeUndefined() + expect((option.baseOption as Record).toolbox).toBeUndefined() + const media = option.media as Array<{ option: Record }> + expect(media[0].option.toolbox).toBeUndefined() + }) + + it('adds no tooltip to a document that declares none', () => { + const option = parse({ schema_version: 1, option: { series: [{ type: 'bar', data: [1] }] } }) + expect('tooltip' in option).toBe(false) + }) + + it('rejects a document too deep to walk instead of throwing', () => { + const nest = (depth: number) => { + let series = '1' + for (let i = 0; i < depth; i++) series = `[${series}]` + return `{"schema_version":1,"option":{"series":${series}}}` + } + expect(parseChartSpec(nest(500)).error).toBeUndefined() + expect(parseChartSpec(nest(50_000)).error).toMatch(/deeply/) + }) + + it('leaves dataset rows alone — they hold data, not components', () => { + const rows = [{ tooltip: 'ok', toolbox: 'ok' }] + const option = parse({ schema_version: 1, option: { dataset: { source: rows } } }) + expect((option.dataset as Record).source).toEqual(rows) + }) +}) + +describe('chart option confinement against echarts', () => { + /** + * Pins the library behavior the confinement relies on: the tooltip's render + * mode is resolved from the single tooltip component, and a tooltip reaching + * that component only through `media` would otherwise default to HTML. Renders + * server-side so the assertion runs without a DOM. + */ + function renderModel(option: Record) { + const chart = echarts.init(null, null, { + renderer: 'svg', + ssr: true, + width: 400, + height: 300, + }) + try { + chart.setOption(buildChartRenderOption({ option, rows: null })) + return chart.getModel() + } finally { + chart.dispose() + } + } + + it('resolves a media-only tooltip to the canvas render mode', () => { + const model = renderModel( + parse({ + schema_version: 1, + option: { + xAxis: {}, + yAxis: {}, + series: [{ type: 'bar', data: [1, 2] }], + media: [{ query: { minWidth: 100 }, option: { tooltip: { formatter: XSS_FORMATTER } } }], + }, + }) + ) + expect(model.getComponent('tooltip')?.get('renderMode')).toBe('richText') + }) + + it.each([ + ['top-level', { tooltip: { formatter: XSS_FORMATTER } }], + [ + 'series-level', + { series: [{ type: 'bar', data: [1], tooltip: { formatter: XSS_FORMATTER } }] }, + ], + ['non-object top-level', { tooltip: 'x', series: [{ type: 'bar', data: [1], tooltip: {} }] }], + ['array', { tooltip: [{ formatter: XSS_FORMATTER }] }], + ['baseOption', { baseOption: { tooltip: { formatter: XSS_FORMATTER } } }], + ])('leaves no tooltip component on the innerHTML path (%s)', (_label, declaration) => { + const model = renderModel( + parse({ + schema_version: 1, + option: { xAxis: {}, yAxis: {}, series: [{ type: 'bar', data: [1] }], ...declaration }, + }) + ) + const renderMode = model.getComponent('tooltip')?.get('renderMode') + expect(renderMode === undefined || renderMode === 'richText').toBe(true) + }) + + it('never instantiates a toolbox component', () => { + const model = renderModel( + parse({ + schema_version: 1, + option: { + xAxis: {}, + yAxis: {}, + series: [{ type: 'bar', data: [1] }], + toolbox: { feature: { dataView: {} } }, + }, + }) + ) + expect(model.getComponent('toolbox')).toBeUndefined() + }) +}) + describe('parseChartSpec table-shaping validation', () => { it('rejects groupBy without aggregate and bad ops', () => { const base = { schema_version: 1, option: {} } diff --git a/apps/sim/lib/charts/spec.ts b/apps/sim/lib/charts/spec.ts index c406cab6071..5d1a6479c86 100644 --- a/apps/sim/lib/charts/spec.ts +++ b/apps/sim/lib/charts/spec.ts @@ -37,9 +37,10 @@ export interface ChartStaticSource { /** * A `.chart` file (`text/x-sim-chart`): a declarative ECharts document. The - * `option` is a plain ECharts option object; `source` optionally supplies the - * data — inline rows, or a live read of a Sim table injected as - * `option.dataset.source` so the chart stays current with the table. + * `option` is a plain ECharts option object, confined to ECharts' canvas render + * paths by {@link confineOptionToCanvas}; `source` optionally supplies the data — + * inline rows, or a live read of a Sim table injected as `option.dataset.source` + * so the chart stays current with the table. */ export interface ChartSpec { schema_version: number @@ -48,6 +49,52 @@ export interface ChartSpec { option: Record } +/** ECharts' tooltip render mode that draws into the chart canvas instead of the DOM. */ +const CANVAS_TOOLTIP_RENDER_MODE = 'richText' + +/** + * Closes the paths by which an ECharts option reaches the DOM, so a `.chart` + * document cannot inject markup into the page that renders it. A document is + * untrusted input: any workspace member authors one, and `/f/` renders it + * to anonymous visitors on the app origin. + * + * ECharts draws through canvas with two exceptions. A `tooltip` left in its + * default `renderMode: 'html'` assigns its content to `el.innerHTML`, and a + * string `formatter` is used as that content's template verbatim — only the + * values substituted into it are escaped. A `toolbox` assigns `dataView.lang` + * entries to `innerHTML` and fills a `saveAsImage` popup with `document.write`. + * Forcing the render mode and dropping the toolbox leaves the document no DOM + * sink at all, which holds whatever any individual option value contains. + * + * The walk is deep because `tooltip` is not only a top-level component: + * `baseOption`, `media[].option`, and timeline `options[]` each carry their own, + * a tooltip declared *only* under `media` still instantiates the component in + * HTML mode once its query matches, and a `media` entry can override a top-level + * `renderMode`. `dataset` is skipped — it holds rows, not components. + */ +function confineOptionToCanvas(node: unknown): void { + if (Array.isArray(node)) { + for (const entry of node) confineOptionToCanvas(entry) + return + } + if (node === null || typeof node !== 'object') return + const record = node as Record + // biome-ignore lint/performance/noDelete: the key must be absent, not undefined-valued + if ('toolbox' in record) delete record.toolbox + for (const key of Object.keys(record)) { + if (key === 'dataset') continue + const value = record[key] + if (key === 'tooltip') { + for (const tooltip of Array.isArray(value) ? value : [value]) { + if (tooltip !== null && typeof tooltip === 'object') { + ;(tooltip as Record).renderMode = CANVAS_TOOLTIP_RENDER_MODE + } + } + } + confineOptionToCanvas(value) + } +} + export function parseChartSpec(content: string): { spec?: ChartSpec; error?: string } { let raw: unknown try { @@ -98,6 +145,17 @@ export function parseChartSpec(content: string): { spec?: ChartSpec; error?: str return { error: '"source.type" must be "static" or "table"' } } } + const option = doc.option as Record + try { + confineOptionToCanvas(option) + } catch { + // Exhausting the stack is the only way the walk fails, and an option that + // deep never reaches the renderer anyway — `structuredClone` in + // `buildChartRenderOption` throws on it too. Rejecting it here shows the + // document's error card instead of failing inside the render. + return { error: 'chart document is nested too deeply' } + } + // Built explicitly from the validated fields — no blanket cast, and no // unvalidated extra keys riding along on the parsed spec. return { @@ -105,7 +163,7 @@ export function parseChartSpec(content: string): { spec?: ChartSpec; error?: str schema_version: 1, title: typeof doc.title === 'string' ? doc.title : undefined, source, - option: doc.option as Record, + option, }, } } diff --git a/apps/sim/lib/pptx-renderer/renderer/chart-renderer.ts b/apps/sim/lib/pptx-renderer/renderer/chart-renderer.ts index a13f43121b2..c0b4a7dae31 100644 --- a/apps/sim/lib/pptx-renderer/renderer/chart-renderer.ts +++ b/apps/sim/lib/pptx-renderer/renderer/chart-renderer.ts @@ -2061,9 +2061,12 @@ function buildBubbleChartOption( : undefined, tooltip: { trigger: 'item', + // The name comes from the uploaded document and lands in the tooltip's innerHTML. + // ECharts escapes the markup it builds itself; a hand-built one escapes its own. formatter: (params: unknown) => { const p = params as { seriesName: string; value: number[] } - return `${p.seriesName}
x: ${p.value[0]}, y: ${p.value[1]}, size: ${p.value[2]}` + const name = echarts.format.encodeHTML(p.seriesName) + return `${name}
x: ${p.value[0]}, y: ${p.value[1]}, size: ${p.value[2]}` }, }, legend: buildLegendOption(