From 1fa6efe07818c4f3891d47706de0c0751acebf09 Mon Sep 17 00:00:00 2001 From: sha174n Date: Thu, 28 May 2026 14:27:00 +0100 Subject: [PATCH 1/2] fix(nvd3): sanitize annotation tooltip HTML content Route the annotation tooltip HTML through DOMPurify before it is passed to d3-tip's .html() sink, matching the sanitization already applied in sibling tooltip helpers in this file. --- .../plugins/legacy-preset-chart-nvd3/src/utils.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/superset-frontend/plugins/legacy-preset-chart-nvd3/src/utils.ts b/superset-frontend/plugins/legacy-preset-chart-nvd3/src/utils.ts index 9fdeeeafa614..3cf660c42fb7 100644 --- a/superset-frontend/plugins/legacy-preset-chart-nvd3/src/utils.ts +++ b/superset-frontend/plugins/legacy-preset-chart-nvd3/src/utils.ts @@ -279,17 +279,19 @@ export function tipFactory(layer) { if (!d) { return ''; } - const title = + const rawTitle = d[layer.titleColumn] && d[layer.titleColumn].length > 0 ? `${d[layer.titleColumn]} - ${layer.name}` : layer.name; - const body = Array.isArray(layer.descriptionColumns) + const rawBody = Array.isArray(layer.descriptionColumns) ? layer.descriptionColumns.map(c => d[c]) : Object.values(d); - return `
${title}

${body.join( - ', ', - )}
`; + return dompurify.sanitize( + `
${rawTitle}

${rawBody.join( + ', ', + )}
`, + ); }); } From d0beb9e695a34e6d337ebd28d6ee0e038acfdab5 Mon Sep 17 00:00:00 2001 From: sha174n Date: Thu, 28 May 2026 15:12:33 +0100 Subject: [PATCH 2/2] fix(charts): sanitize tooltip HTML across nvd3, rose and partition plugins Expand the SEC-125 cluster's annotation-tooltip fix to cover every confirmed tooltip HTML sink in the cluster, plus the default nvd3 tooltip path that shares the same root cause. - legacy-preset-chart-nvd3/src/utils.ts: wrap generateBubbleTooltipContent, generateMultiLineTooltipContent, and wrapTooltip output with DOMPurify; flip getFormattedKey shouldDompurify to true for consistency. - legacy-plugin-chart-rose/src/Rose.ts: sanitize series keys in tooltipData and legendData via sanitizeHtml. - legacy-plugin-chart-partition/src/Partition.ts: sanitize tooltip HTML with sanitizeHtml(t) before tip.html(t). - legacy-preset-chart-nvd3/test/utils.test.ts: regression tests for the three nvd3 utils helpers (script and img onerror payloads). - scripts/oxlint.sh: drive-by; last line was '[ -n "$output" ] && echo "$output"' which under 'set -e' aborts the script when oxlint succeeds with empty output. Replaced with an explicit 'if [ -n "$output" ]; then ... fi' so a clean lint run no longer fails the pre-commit hook. Co-Authored-By: Claude Opus 4.7 --- scripts/oxlint.sh | 4 +- .../src/Partition.ts | 3 +- .../legacy-plugin-chart-rose/src/Rose.ts | 15 ++- .../legacy-preset-chart-nvd3/src/utils.ts | 21 ++-- .../test/utils.test.ts | 95 +++++++++++++++++++ 5 files changed, 125 insertions(+), 13 deletions(-) diff --git a/scripts/oxlint.sh b/scripts/oxlint.sh index 95f48afabb6b..604acae86c33 100755 --- a/scripts/oxlint.sh +++ b/scripts/oxlint.sh @@ -55,7 +55,9 @@ if [ ${#js_ts_files[@]} -gt 0 ]; then echo "$output" >&2 exit 1 } - [ -n "$output" ] && echo "$output" + if [ -n "$output" ]; then + echo "$output" + fi else echo "No JavaScript/TypeScript files to lint" fi diff --git a/superset-frontend/plugins/legacy-plugin-chart-partition/src/Partition.ts b/superset-frontend/plugins/legacy-plugin-chart-partition/src/Partition.ts index db5d47726a88..cd57ecc22e88 100644 --- a/superset-frontend/plugins/legacy-plugin-chart-partition/src/Partition.ts +++ b/superset-frontend/plugins/legacy-plugin-chart-partition/src/Partition.ts @@ -27,6 +27,7 @@ import { getNumberFormatter, getTimeFormatter, CategoricalColorNamespace, + sanitizeHtml, } from '@superset-ui/core'; interface PartitionDataNode { @@ -345,7 +346,7 @@ function Icicle(element: HTMLElement, props: IcicleProps): void { t += ''; const [tipX, tipY] = d3.mouse(element); tip - .html(t) + .html(sanitizeHtml(t)) .style('left', `${tipX + 15}px`) .style('top', `${tipY}px`); } diff --git a/superset-frontend/plugins/legacy-plugin-chart-rose/src/Rose.ts b/superset-frontend/plugins/legacy-plugin-chart-rose/src/Rose.ts index 32a3242bbc38..14a5a655ccdf 100644 --- a/superset-frontend/plugins/legacy-plugin-chart-rose/src/Rose.ts +++ b/superset-frontend/plugins/legacy-plugin-chart-rose/src/Rose.ts @@ -27,6 +27,7 @@ import { getTimeFormatter, getNumberFormatter, CategoricalColorNamespace, + sanitizeHtml, } from '@superset-ui/core'; interface RoseDataEntry { @@ -146,24 +147,32 @@ function Rose(element: HTMLElement, props: RoseProps): void { function legendData(adatum: RoseData) { return adatum[times[0]].map((v: RoseDataEntry, i: number) => ({ disabled: state.disabled[i], - key: v.name, + // nvd3-fork's legend currently renders `key` via .text(), so raw + // markup would be escaped today. Sanitize at the data boundary + // anyway: it makes the safety property a local invariant rather + // than depending on the vendored legend's render choice. + key: sanitizeHtml(v.name), })); } function tooltipData(d: ArcDatum, i: number, adatum: RoseData) { const timeIndex = Math.floor(d.arcId / numGroups); + // nvd3-fork's nv.models.tooltip renders the `key` strings via .html(), + // so any HTML in user-controlled column values would execute. Pass the + // keys through sanitizeHtml to strip dangerous markup while preserving + // legitimate text content. const series = useRichTooltip ? adatum[times[timeIndex]] .filter(v => !state.disabled[v.id % numGroups]) .map(v => ({ - key: v.name, + key: sanitizeHtml(v.name), value: v.value, color: colorFn(v.name, sliceId), highlight: v.id === d.arcId, })) : [ { - key: d.name, + key: sanitizeHtml(d.name), value: d.val, color: colorFn(d.name, sliceId), }, diff --git a/superset-frontend/plugins/legacy-preset-chart-nvd3/src/utils.ts b/superset-frontend/plugins/legacy-preset-chart-nvd3/src/utils.ts index 3cf660c42fb7..4474d50672e3 100644 --- a/superset-frontend/plugins/legacy-preset-chart-nvd3/src/utils.ts +++ b/superset-frontend/plugins/legacy-preset-chart-nvd3/src/utils.ts @@ -152,7 +152,7 @@ export function generateMultiLineTooltipContent(d, xFormatter, yFormatters) { d.series.forEach((series, i) => { const yFormatter = yFormatters[i]; - const key = getFormattedKey(series.key, false); + const key = getFormattedKey(series.key, true); tooltip += "" + `
` + @@ -162,7 +162,7 @@ export function generateMultiLineTooltipContent(d, xFormatter, yFormatters) { tooltip += ''; - return tooltip; + return dompurify.sanitize(tooltip); } export function generateTimePivotTooltip(d, xFormatter, yFormatter) { @@ -223,7 +223,7 @@ export function generateBubbleTooltipContent({ s += createHTMLRow(getLabel(sizeField), sizeFormatter(point.size)); s += ''; - return s; + return dompurify.sanitize(s); } // shouldRemove indicates whether the nvtooltips should be removed from the DOM @@ -262,11 +262,16 @@ export function wrapTooltip(chart) { : chart; const tooltipGeneratorFunc = tooltipLayer.tooltip.contentGenerator(); tooltipLayer.tooltip.contentGenerator(d => { - let tooltip = `
`; - tooltip += tooltipGeneratorFunc(d); - tooltip += '
'; - - return tooltip; + // The nvd3-fork default contentGenerator builds tooltip HTML with + // unescaped series keys (and feeds them into the tooltip's `.html()` + // sink at render time). Run the final string through DOMPurify so + // charts that do NOT install a custom contentGenerator (Line, Bar, + // Area, Pie, BoxPlot, etc.) cannot execute stored payloads in + // column or series names. Custom contentGenerators set elsewhere in + // this module already return sanitized output, making this a + // belt-and-braces wrap. + const tooltip = `
${tooltipGeneratorFunc(d)}
`; + return dompurify.sanitize(tooltip); }); } diff --git a/superset-frontend/plugins/legacy-preset-chart-nvd3/test/utils.test.ts b/superset-frontend/plugins/legacy-preset-chart-nvd3/test/utils.test.ts index a883d4c6b7f4..a20fa38411c5 100644 --- a/superset-frontend/plugins/legacy-preset-chart-nvd3/test/utils.test.ts +++ b/superset-frontend/plugins/legacy-preset-chart-nvd3/test/utils.test.ts @@ -24,8 +24,11 @@ import { import { computeYDomain, + generateBubbleTooltipContent, + generateMultiLineTooltipContent, getTimeOrNumberFormatter, formatLabel, + tipFactory, } from '../src/utils'; const DATA = [ @@ -181,4 +184,96 @@ describe('nvd3/utils', () => { ]); }); }); + + // ------------------------------------------------------------------ + // Tooltip HTML sanitisation (XSS regression). + // Each helper below feeds user-controlled column values into a + // d3 / nvd3 .html() sink; the sanitised return must strip dangerous + // markup so a stored payload cannot execute on hover. + // ------------------------------------------------------------------ + + describe('generateBubbleTooltipContent() sanitises user input', () => { + test('strips ', + x: 1, + y: 2, + size: 3, + }, + entity: 'entity', + xField: 'x', + yField: 'y', + sizeField: 'size', + xFormatter: (v: number) => String(v), + yFormatter: (v: number) => String(v), + sizeFormatter: (v: number) => String(v), + }); + expect(html).not.toMatch(/', + color: 'red', + value: 1, + }, + ], + }, + (v: number) => String(v), + [(v: number) => String(v)], + ); + expect(html).not.toMatch(/payload', + }; + const html = tip.html()(datum); + expect(html).not.toMatch(/