Skip to content
Open
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
4 changes: 3 additions & 1 deletion scripts/oxlint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
getNumberFormatter,
getTimeFormatter,
CategoricalColorNamespace,
sanitizeHtml,
} from '@superset-ui/core';

interface PartitionDataNode {
Expand Down Expand Up @@ -345,7 +346,7 @@ function Icicle(element: HTMLElement, props: IcicleProps): void {
t += '</tbody></table>';
const [tipX, tipY] = d3.mouse(element);
tip
.html(t)
.html(sanitizeHtml(t))
.style('left', `${tipX + 15}px`)
.style('top', `${tipY}px`);
}
Expand Down
15 changes: 12 additions & 3 deletions superset-frontend/plugins/legacy-plugin-chart-rose/src/Rose.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
getTimeFormatter,
getNumberFormatter,
CategoricalColorNamespace,
sanitizeHtml,
} from '@superset-ui/core';

interface RoseDataEntry {
Expand Down Expand Up @@ -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),
},
Expand Down
33 changes: 20 additions & 13 deletions superset-frontend/plugins/legacy-preset-chart-nvd3/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 +=
"<tr><td class='legend-color-guide'>" +
`<div style="background-color: ${series.color};"></div></td>` +
Expand All @@ -162,7 +162,7 @@ export function generateMultiLineTooltipContent(d, xFormatter, yFormatters) {

tooltip += '</tbody></table>';

return tooltip;
return dompurify.sanitize(tooltip);
}

export function generateTimePivotTooltip(d, xFormatter, yFormatter) {
Expand Down Expand Up @@ -223,7 +223,7 @@ export function generateBubbleTooltipContent({
s += createHTMLRow(getLabel(sizeField), sizeFormatter(point.size));
s += '</table>';

return s;
return dompurify.sanitize(s);
}

// shouldRemove indicates whether the nvtooltips should be removed from the DOM
Expand Down Expand Up @@ -262,11 +262,16 @@ export function wrapTooltip(chart) {
: chart;
const tooltipGeneratorFunc = tooltipLayer.tooltip.contentGenerator();
tooltipLayer.tooltip.contentGenerator(d => {
let tooltip = `<div>`;
tooltip += tooltipGeneratorFunc(d);
tooltip += '</div>';

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 = `<div>${tooltipGeneratorFunc(d)}</div>`;
return dompurify.sanitize(tooltip);
});
}

Expand All @@ -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 `<div><strong>${title}</strong></div><br/><div>${body.join(
', ',
)}</div>`;
return dompurify.sanitize(
`<div><strong>${rawTitle}</strong></div><br/><div>${rawBody.join(
', ',
)}</div>`,
);
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,11 @@ import {

import {
computeYDomain,
generateBubbleTooltipContent,
generateMultiLineTooltipContent,
getTimeOrNumberFormatter,
formatLabel,
tipFactory,
} from '../src/utils';

const DATA = [
Expand Down Expand Up @@ -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 <script> from the entity column', () => {
const html = generateBubbleTooltipContent({
point: {
color: 'red',
group: 'g',
entity: '<script>alert(1)</script>',
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(/<script/i);
expect(html).not.toMatch(/onerror=/i);
});

test('strips <img onerror> injected via the group column', () => {
const html = generateBubbleTooltipContent({
point: {
color: 'red',
group: '<img src=x onerror=alert(1)>',
entity: 'safe',
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(/onerror/i);
});
});

describe('generateMultiLineTooltipContent() sanitises user input', () => {
test('strips <script> from a series key', () => {
const html = generateMultiLineTooltipContent(
{
value: 0,
series: [
{
key: '<script>alert(1)</script>',
color: 'red',
value: 1,
},
],
},
(v: number) => String(v),
[(v: number) => String(v)],
);
expect(html).not.toMatch(/<script/i);
});
});

describe('tipFactory() sanitises annotation columns', () => {
test('strips <script> from a description column value', () => {
const tip = tipFactory({
annotationTipClass: 'foo',
titleColumn: 'title',
descriptionColumns: ['desc'],
name: 'layer',
});
// d3-tip's .html(fn) stores the callback as the renderer; invoke
// it directly to assert the sanitised output.
const datum = {
title: 'normal',
desc: '<script>alert(1)</script>payload',
};
const html = tip.html()(datum);
expect(html).not.toMatch(/<script/i);
expect(html).toContain('payload');
});
});
});