Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 37 additions & 1 deletion apps/sim/lib/charts/spec.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ describe('shapeTableRows', () => {
})

const XSS_FORMATTER = '<img src=x onerror="alert(1)">'
const XSS_LINK = 'javascript:alert(document.domain)'

describe('parseChartSpec option confinement', () => {
it('forces the tooltip off the innerHTML path, keeping the formatter template', () => {
Expand Down Expand Up @@ -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)
Expand All @@ -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<string, unknown>).source).toEqual(rows)
})
Expand Down Expand Up @@ -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', () => {
Expand Down
17 changes: 14 additions & 3 deletions apps/sim/lib/charts/spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -79,8 +89,9 @@ function confineOptionToCanvas(node: unknown): void {
}
if (node === null || typeof node !== 'object') return
const record = node as Record<string, unknown>
// 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]
Expand Down
Loading