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 9fdeeeafa614..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); }); } @@ -279,17 +284,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( + ', ', + )}
`, + ); }); } 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(/