diff --git a/apps/sim/lib/charts/spec.test.ts b/apps/sim/lib/charts/spec.test.ts index c0bf9cd2046..1d708ede5e1 100644 --- a/apps/sim/lib/charts/spec.test.ts +++ b/apps/sim/lib/charts/spec.test.ts @@ -95,6 +95,7 @@ describe('shapeTableRows', () => { }) const XSS_FORMATTER = '' +const XSS_LINK = 'javascript:alert(document.domain)' describe('parseChartSpec option confinement', () => { it('forces the tooltip off the innerHTML path, keeping the formatter template', () => { @@ -175,6 +176,23 @@ describe('parseChartSpec option confinement', () => { expect(media[0].option.toolbox).toBeUndefined() }) + it('drops every navigation sink — title link/sublink and treemap/sunburst item links', () => { + const option = parse({ + schema_version: 1, + option: { + title: { text: 'click me', link: XSS_LINK, sublink: XSS_LINK, target: 'self' }, + series: [ + { type: 'treemap', data: [{ name: 'a', value: 1, link: XSS_LINK }] }, + { type: 'sunburst', data: [{ name: 'b', value: 1, link: XSS_LINK }] }, + ], + baseOption: { title: { link: XSS_LINK } }, + media: [{ query: { minWidth: 100 }, option: { title: { link: XSS_LINK } } }], + }, + }) + expect(JSON.stringify(option)).not.toContain('javascript:') + expect(option.title).toEqual({ text: 'click me', target: 'self' }) + }) + 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) @@ -191,7 +209,7 @@ describe('parseChartSpec option confinement', () => { }) it('leaves dataset rows alone — they hold data, not components', () => { - const rows = [{ tooltip: 'ok', toolbox: 'ok' }] + const rows = [{ tooltip: 'ok', toolbox: 'ok', link: 'ok' }] const option = parse({ schema_version: 1, option: { dataset: { source: rows } } }) expect((option.dataset as Record).source).toEqual(rows) }) @@ -268,6 +286,24 @@ describe('chart option confinement against echarts', () => { ) expect(model.getComponent('toolbox')).toBeUndefined() }) + + it('leaves the title component no link to hand to windowOpen', () => { + const model = renderModel( + parse({ + schema_version: 1, + option: { + xAxis: {}, + yAxis: {}, + series: [{ type: 'bar', data: [1] }], + title: { text: 'click me', link: XSS_LINK, sublink: XSS_LINK }, + }, + }) + ) + const title = model.getComponent('title') + expect(title?.get('text')).toBe('click me') + expect(title?.get('link')).toBeUndefined() + expect(title?.get('sublink')).toBeUndefined() + }) }) describe('parseChartSpec table-shaping validation', () => { diff --git a/apps/sim/lib/charts/spec.ts b/apps/sim/lib/charts/spec.ts index 5d1a6479c86..37bc55f0aad 100644 --- a/apps/sim/lib/charts/spec.ts +++ b/apps/sim/lib/charts/spec.ts @@ -52,6 +52,9 @@ export interface ChartSpec { /** ECharts' tooltip render mode that draws into the chart canvas instead of the DOM. */ const CANVAS_TOOLTIP_RENDER_MODE = 'richText' +/** Option keys stripped at every level: the `toolbox` DOM sink and the `link`/`sublink` navigation sinks. */ +const DROPPED_KEYS = ['toolbox', 'link', 'sublink'] as const + /** * 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 @@ -63,7 +66,14 @@ const CANVAS_TOOLTIP_RENDER_MODE = 'richText' * 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 + * Forcing the render mode and dropping the toolbox leaves it no DOM sink. + * + * ECharts also navigates: `title.link`, `title.sublink`, and a `link` on a + * treemap or sunburst data item each reach `windowOpen`, which assigns the URL + * to `location.href` — so a `javascript:` URL runs on this origin on a single + * click. A chart has no reason to navigate its viewer, so the keys are dropped + * everywhere rather than scheme-checked, which would still leave an open + * redirect on an authenticated origin. Between them the document is left no * sink at all, which holds whatever any individual option value contains. * * The walk is deep because `tooltip` is not only a top-level component: @@ -79,8 +89,9 @@ function confineOptionToCanvas(node: unknown): void { } 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 DROPPED_KEYS) { + if (key in record) delete record[key] + } for (const key of Object.keys(record)) { if (key === 'dataset') continue const value = record[key]