diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx index b272403df0c..8698f26f4bd 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx @@ -51,6 +51,20 @@ export interface MetricOptionProps { shouldShowTooltip?: boolean; } +/** + * `url` is an arbitrary caller-supplied string rendered as an href. Only + * http(s) and relative URLs become links; other schemes degrade to plain + * text. + */ +function isSafeHref(url: string): boolean { + try { + const { protocol } = new URL(url, window.location.origin); + return protocol === 'http:' || protocol === 'https:'; + } catch { + return false; + } +} + export function MetricOption({ metric, labelRef, @@ -70,7 +84,7 @@ export function MetricOption({ `} ref={labelRef} > - {url ? ( + {url && isSafeHref(url) ? ( { const { getByTestId } = setup(); expect(getByTestId('mock-tooltip')).toBeInTheDocument(); }); +test('does not render javascript: URLs as links', () => { + // Regression test: the url prop can be creator-authored and must + // never become a script-bearing href for other viewers. + const { queryByRole, getByText } = setup({ + url: 'javascript:alert(document.domain)', // eslint-disable-line no-script-url + }); + expect(queryByRole('link')).not.toBeInTheDocument(); + expect(getByText(defaultProps.metric.verbose_name)).toBeInTheDocument(); +}); +test('does not render data: URLs as links', () => { + const { queryByRole } = setup({ + url: 'data:text/html,', + }); + expect(queryByRole('link')).not.toBeInTheDocument(); +}); +test('renders relative URLs as links', () => { + const { getByRole } = setup({ + url: '/superset/dashboard/1/', + }); + expect( + getByRole('link', { name: defaultProps.metric.verbose_name }), + ).toHaveAttribute('href', '/superset/dashboard/1/'); +}); diff --git a/superset-frontend/plugins/plugin-chart-calendar/src/utils.ts b/superset-frontend/plugins/plugin-chart-calendar/src/utils.ts index ff271cc73f7..a0570e01db3 100644 --- a/superset-frontend/plugins/plugin-chart-calendar/src/utils.ts +++ b/superset-frontend/plugins/plugin-chart-calendar/src/utils.ts @@ -38,3 +38,11 @@ export const convertUTCTimestampToLocal = (utcTimestamp: number): number => { const offsetMs = date.getTimezoneOffset() * 60 * 1000; return utcTimestamp + offsetMs; }; + +// Escapes HTML special characters before formatter output reaches an +// innerHTML sink. Mirrors plugin-chart-country-map's escapeHtml. +export const escapeHtml = (text: unknown): string => { + const div = document.createElement('div'); + div.textContent = String(text); + return div.innerHTML; +}; diff --git a/superset-frontend/plugins/plugin-chart-calendar/src/vendor/cal-heatmap.ts b/superset-frontend/plugins/plugin-chart-calendar/src/vendor/cal-heatmap.ts index a040ab19756..8b366be1838 100644 --- a/superset-frontend/plugins/plugin-chart-calendar/src/vendor/cal-heatmap.ts +++ b/superset-frontend/plugins/plugin-chart-calendar/src/vendor/cal-heatmap.ts @@ -13,6 +13,7 @@ import d3tip from 'd3-tip'; import { t } from '@apache-superset/core/translation'; import { getContrastingColor } from '@superset-ui/core'; import { CALENDAR_TOOLTIP_CLASS } from '../tooltip'; +import { escapeHtml } from '../utils'; var d3 = typeof require === 'function' ? require('d3') : window.d3; @@ -22,14 +23,16 @@ var CalHeatMap = function () { 'use strict'; var self = this; + // d3-tip assigns the .html() return value to the tip node via + // innerHTML, so formatter output is HTML-escaped first. self.tip = d3tip() .attr('class', `d3-tip ${CALENDAR_TOOLTIP_CLASS}`) .direction('n') .offset([-5, 0]) .html( d => ` - ${self.options.timeFormatter(d.t)}: ${self.options.valueFormatter( - d.v, + ${escapeHtml(self.options.timeFormatter(d.t))}: ${escapeHtml( + self.options.valueFormatter(d.v), )} `, ); @@ -37,7 +40,7 @@ var CalHeatMap = function () { .attr('class', `d3-tip ${CALENDAR_TOOLTIP_CLASS}`) .direction('n') .offset([-5, 0]) - .html(d => self.options.valueFormatter(d)); + .html(d => escapeHtml(self.options.valueFormatter(d))); this.allowedDataType = ['json', 'csv', 'tsv', 'txt']; diff --git a/superset-frontend/plugins/plugin-chart-calendar/test/cal-heatmap.test.ts b/superset-frontend/plugins/plugin-chart-calendar/test/cal-heatmap.test.ts index 78f6efc6b72..2247592a50e 100644 --- a/superset-frontend/plugins/plugin-chart-calendar/test/cal-heatmap.test.ts +++ b/superset-frontend/plugins/plugin-chart-calendar/test/cal-heatmap.test.ts @@ -25,8 +25,12 @@ type FunctionalDateFormat = (date: Date) => string; interface CalHeatMapInstance { options: { dateFormatter: DateFormatter | null; + timeFormatter: (t: number) => string; + valueFormatter: (v: number) => string; }; formatDate(date: Date, format: string | FunctionalDateFormat): string; + tip: { html(): (d: { t: number; v: number }) => string }; + legendTip: { html(): (d: number) => string }; } const CalHeatMap = CalHeatMapImport as unknown as new () => CalHeatMapInstance; @@ -59,3 +63,29 @@ test('CalHeatMap keeps the D3 formatter fallback', () => { expect(calendar.formatDate(date, '%B')).toBe('January'); }); + +test('cell tooltip HTML escapes creator-controlled formatter output', () => { + // Regression test: the tip's .html() callback is assigned to the + // tooltip node via innerHTML (d3-tip), so formatter output must be + // escaped before it's returned. + const calendar = new CalHeatMap(); + calendar.options.timeFormatter = () => ''; + calendar.options.valueFormatter = () => ''; + + const html = calendar.tip.html()({ t: 0, v: 1 }); + + expect(html).not.toContain(' { + const calendar = new CalHeatMap(); + calendar.options.valueFormatter = () => ''; + + const html = calendar.legendTip.html()(1); + + expect(html).not.toContain(' { const utcTimestamp = 1420070400000; // 2015-01-01 00:00:00 UTC @@ -87,3 +91,22 @@ test('convertUTCTimestampToLocal and getFormattedUTCTime work together to displa const formattedTime = getFormattedUTCTime(localTimestamp, '%Y-%m-%d'); expect(formattedTime).toContain('2024-01-01'); }); + +test('escapeHtml neutralizes markup smuggled through a time format string', () => { + // Regression test: d3-time-format passes non-% characters through + // verbatim, so escaping must happen before the innerHTML sink. + const formatted = getFormattedUTCTime( + 1704067200000, + '%Y ', + ); + const escaped = escapeHtml(formatted); + + expect(formatted).toContain(' { + expect(escapeHtml(1234)).toEqual('1234'); + expect(escapeHtml('a & b < c')).toEqual('a & b < c'); +}); diff --git a/superset-frontend/plugins/plugin-chart-cartodiagram/src/util/layerUtil.tsx b/superset-frontend/plugins/plugin-chart-cartodiagram/src/util/layerUtil.tsx index 4af5066efca..25084f69223 100644 --- a/superset-frontend/plugins/plugin-chart-cartodiagram/src/util/layerUtil.tsx +++ b/superset-frontend/plugins/plugin-chart-cartodiagram/src/util/layerUtil.tsx @@ -33,6 +33,27 @@ import { WmsLayerConf, WfsLayerConf, LayerConf, XyzLayerConf } from '../types'; import { isWfsLayerConf, isWmsLayerConf, isXyzLayerConf } from '../typeguards'; import { isVersionBelow } from './serviceUtil'; +/** + * Escape HTML special characters in a layer attribution string. + * + * OpenLayers' Attribution control renders attribution strings via innerHTML, + * and the attribution here comes from creator-supplied chart form data, so it + * must be treated as untrusted text rather than markup to prevent stored XSS. + * + * @param attribution The attribution string from the layer configuration + * + * @returns The attribution with HTML special characters escaped + */ +export const escapeAttribution = (attribution?: string): string | undefined => + attribution === undefined + ? undefined + : attribution + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); + /** * Create a WMS layer. * @@ -49,7 +70,7 @@ export const createWmsLayer = (wmsLayerConf: WmsLayerConf) => { LAYERS: layersParam, VERSION: version, }, - attributions: attribution, + attributions: escapeAttribution(attribution), }), }); }; @@ -66,7 +87,7 @@ export const createXyzLayer = (xyzLayerConf: XyzLayerConf) => { return new TileLayer({ source: new XyzSource({ url, - attributions: attribution, + attributions: escapeAttribution(attribution), }), }); }; @@ -90,7 +111,7 @@ export const createWfsLayer = async (wfsLayerConf: WfsLayerConf) => { const wfsSource = new VectorSource({ format: new GeoJSON(), - attributions: attribution, + attributions: escapeAttribution(attribution), url: extent => { const requestUrl = new URL(url); const params = requestUrl.searchParams; diff --git a/superset-frontend/plugins/plugin-chart-cartodiagram/test/util/layerUtil.test.ts b/superset-frontend/plugins/plugin-chart-cartodiagram/test/util/layerUtil.test.ts index a4141d89543..033cfbe059d 100644 --- a/superset-frontend/plugins/plugin-chart-cartodiagram/test/util/layerUtil.test.ts +++ b/superset-frontend/plugins/plugin-chart-cartodiagram/test/util/layerUtil.test.ts @@ -17,20 +17,65 @@ * under the License. */ -import { WfsLayerConf } from '../../src/types'; +import { WfsLayerConf, WmsLayerConf, XyzLayerConf } from '../../src/types'; import { createLayer, createWfsLayer, createWmsLayer, createXyzLayer, + escapeAttribution, } from '../../src/util/layerUtil'; describe('layerUtil', () => { + describe('escapeAttribution', () => { + test('escapes HTML markup in attribution strings', () => { + expect(escapeAttribution('(c) OSM ')).toBe( + '(c) OSM <img src=x onerror=alert(1)>', + ); + expect(escapeAttribution('a & "b" \'c\'')).toBe( + 'a & "b" 'c'', + ); + expect(escapeAttribution(undefined)).toBeUndefined(); + }); + }); + describe('createWmsLayer', () => { test('exists', () => { // function is trivial expect(createWmsLayer).toBeDefined(); }); + + test('escapes HTML in the layer attribution', () => { + const wmsLayerConf: WmsLayerConf = { + title: 'wms', + type: 'WMS', + url: 'https://ows-demo.terrestris.de/geoserver/osm/wms', + version: '1.3.0', + layersParam: 'osm:osm-fuel', + attribution: '(c) OSM ', + }; + const layer = createWmsLayer(wmsLayerConf); + const attributions = layer.getSource()?.getAttributions(); + expect(attributions?.(undefined as never)).toEqual([ + '(c) OSM <img src=x onerror=alert(1)>', + ]); + }); + }); + + describe('createXyzLayer', () => { + test('escapes HTML in the layer attribution', () => { + const xyzLayerConf: XyzLayerConf = { + title: 'osm', + type: 'XYZ', + url: 'https://tile.openstreetmap.org/{z}/{x}/{y}.png', + attribution: '(c) OSM ', + }; + const layer = createXyzLayer(xyzLayerConf); + const attributions = layer.getSource()?.getAttributions(); + expect(attributions?.(undefined as never)).toEqual([ + '(c) OSM <img src=x onerror=alert(1)>', + ]); + }); }); describe('createWfsLayer', () => { diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Radar/utils.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Radar/utils.ts index aeaefd96c02..4ce0d5ef0e8 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Radar/utils.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Radar/utils.ts @@ -18,6 +18,7 @@ */ import { t } from '@apache-superset/core/translation'; import { NumberFormatter } from '@superset-ui/core'; +import { sanitizeHtml } from '../utils/series'; /* function for finding the max metric values among all series data for Radar Chart @@ -63,7 +64,7 @@ export const renderNormalizedTooltip = ( const { color, name = '', value: values } = params; const seriesName = name || 'series0'; - const colorDot = ``; + const colorDot = ``; // Get metric values with denormalization if needed const metricValues: TooltipMetricValue[] = metrics.map((metric, index) => { @@ -85,19 +86,26 @@ export const renderNormalizedTooltip = ( }; }); + // Tooltip is rendered via innerHTML (ECharts default renderMode + // 'html'), so seriesName/metric/value/color are HTML-escaped, matching + // the treatment every other echarts tooltip path applies. const tooltipRows = metricValues .map( ({ metric, value }) => `
-
${colorDot}${metric}:
-
${value}
+
${colorDot}${sanitizeHtml(metric)}:
+
${sanitizeHtml( + String(value), + )}
`, ) .join(''); return ` -
${seriesName}
+
${sanitizeHtml( + seriesName, + )}
${tooltipRows} `; }; diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/eChartOptionsSchema.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/eChartOptionsSchema.ts index f8318dd8577..2410aff53cb 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/eChartOptionsSchema.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/eChartOptionsSchema.ts @@ -28,6 +28,7 @@ */ import { z } from 'zod'; +import { sanitizeHtml } from '@superset-ui/core'; // ============================================================================= // Common Schemas @@ -57,6 +58,33 @@ const fontStyleSchema = z.enum(['normal', 'italic', 'oblique']); /** Symbol type */ const symbolTypeSchema = z.string(); +/** + * With the ECharts default renderMode 'html', a string tooltip formatter is + * assigned to the tooltip DOM element via innerHTML. ECharts formatter + * strings commonly rely on inline markup (e.g. '{b}
{c}') for layout, so + * rejecting every '<' would break that supported usage; instead the value is + * run through the same allowlist sanitizer used for other tooltip HTML, + * which keeps presentational tags and strips anything else. + */ +const sanitizedFormatterSchema = z + .string() + .transform(value => sanitizeHtml(value)); + +/** + * ECharts navigates to title.link/sublink on click, so restrict them to + * http(s) and same-origin relative paths. + */ +const safeLinkSchema = z + .string() + .refine( + value => + /^https?:\/\//i.test(value) || + (value.startsWith('/') && !value.startsWith('//')), + { + message: 'Only http(s) or same-origin relative URLs are allowed', + }, + ); + // ============================================================================= // Text Style Schema // ============================================================================= @@ -168,11 +196,11 @@ export const titleSchema = z.object({ id: z.string().optional(), show: z.boolean().optional(), text: z.string().optional(), - link: z.string().optional(), + link: safeLinkSchema.optional(), target: z.enum(['self', 'blank']).optional(), textStyle: textStyleSchema.optional(), subtext: z.string().optional(), - sublink: z.string().optional(), + sublink: safeLinkSchema.optional(), subtarget: z.enum(['self', 'blank']).optional(), subtextStyle: textStyleSchema.optional(), textAlign: z.enum(['left', 'center', 'right']).optional(), @@ -386,7 +414,9 @@ export const tooltipSchema = z.object({ z.array(z.union([z.number(), z.string()])), ]) .optional(), - formatter: z.string().optional(), // Only string formatters + // Only string formatters: a string tooltip formatter is rendered via + // innerHTML (default renderMode 'html'), so it is sanitized above. + formatter: sanitizedFormatterSchema.optional(), padding: z.union([z.number(), z.array(z.number())]).optional(), backgroundColor: colorSchema.optional(), borderColor: colorSchema.optional(), @@ -397,7 +427,9 @@ export const tooltipSchema = z.object({ shadowOffsetX: z.number().optional(), shadowOffsetY: z.number().optional(), textStyle: textStyleSchema.optional(), - extraCssText: z.string().optional(), + // `extraCssText` is intentionally not accepted; unknown keys are + // stripped by the schema, so configs that still carry it keep working + // minus the raw CSS. order: z .enum(['seriesAsc', 'seriesDesc', 'valueAsc', 'valueDesc']) .optional(), @@ -575,6 +607,9 @@ export const seriesSchema = z.object({ polarIndex: z.number().optional(), geoIndex: z.number().optional(), calendarIndex: z.number().optional(), + // Per-series `tooltip` is intentionally not admitted; the schema + // strips unknown keys. If per-series tooltips are ever admitted, reuse + // tooltipSchema so the formatter sanitization applies. label: labelSchema.optional(), labelLine: z .object({ diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/safeEChartOptionsParser.test.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/safeEChartOptionsParser.test.ts index 59c351f328d..4ee0054c193 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/safeEChartOptionsParser.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/safeEChartOptionsParser.test.ts @@ -523,3 +523,73 @@ test('EChartOptionsParseError contains validation error details', () => { ); } }); + +// ============================================================================= +// Creator-authored options must not reach the tooltip's innerHTML/ +// navigation sinks unsanitized. +// ============================================================================= + +test('sanitizes tooltip string formatters instead of rejecting all markup', () => { + const input = `{ tooltip: { formatter: '' } }`; + const result = parseEChartOptions(input); + + expect(result.success).toBe(true); + expect(result.data?.tooltip).toEqual({ formatter: '' }); +}); + +test('keeps presentational tags in tooltip string formatters', () => { + const input = `{ tooltip: { formatter: '{b}
{c}' } }`; + const result = parseEChartOptions(input); + + expect(result.success).toBe(true); + expect(result.data?.tooltip).toEqual({ formatter: '{b}
{c}' }); +}); + +test('strips per-series tooltip config so its formatter never reaches the merge', () => { + const result = parseEChartOptions( + `{ series: [{ type: 'line', tooltip: { formatter: 'x' } }] }`, + ); + + expect(result.success).toBe(true); + expect(result.data).toEqual({ series: [{ type: 'line' }] }); +}); + +test('accepts markup-free tooltip placeholder formatters', () => { + const input = `{ tooltip: { formatter: '{b}: {c}' } }`; + const result = parseEChartOptions(input); + + expect(result.success).toBe(true); + expect(result.data).toEqual({ tooltip: { formatter: '{b}: {c}' } }); +}); + +test('rejects javascript: URLs in title link and sublink', () => { + expect(() => + parseEChartOptions(`{ title: { link: 'javascript:alert(1)' } }`), + ).toThrow(EChartOptionsParseError); + expect(() => + parseEChartOptions(`{ title: { sublink: 'javascript:alert(1)' } }`), + ).toThrow(EChartOptionsParseError); + expect(() => + parseEChartOptions(`{ title: { link: '//evil.example/x' } }`), + ).toThrow(EChartOptionsParseError); +}); + +test('accepts http(s) and same-origin relative title links', () => { + const result = parseEChartOptions( + `{ title: { link: 'https://superset.apache.org', sublink: '/dashboard/1/' } }`, + ); + + expect(result.success).toBe(true); + expect(result.data).toEqual({ + title: { link: 'https://superset.apache.org', sublink: '/dashboard/1/' }, + }); +}); + +test('strips tooltip extraCssText instead of passing raw CSS through', () => { + const result = parseEChartOptions( + `{ tooltip: { show: true, extraCssText: 'background:url(//evil.example/x)' } }`, + ); + + expect(result.success).toBe(true); + expect(result.data).toEqual({ tooltip: { show: true } }); +}); diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Radar/utils.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Radar/utils.test.ts index c333256d4b7..c870fc0d357 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/Radar/utils.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/Radar/utils.test.ts @@ -70,4 +70,42 @@ describe('renderNormalizedTooltip', () => { expect(tooltip).toContain('N/A'); expect(tooltip).not.toContain('NaN'); }); + + test('should HTML-escape series names from query data', () => { + // Regression test: the tooltip is rendered via innerHTML, so markup + // in query-result values must not become live DOM. + const tooltip = renderNormalizedTooltip( + { ...params, name: '' }, + metrics, + mockGetDenormalizedValue, + metricsWithCustomBounds, + ); + expect(tooltip).not.toContain(' { + const tooltip = renderNormalizedTooltip( + params, + ['', 'metric2'], + mockGetDenormalizedValue, + metricsWithCustomBounds, + ); + expect(tooltip).not.toContain(' { + // Regression test: `color` is interpolated into a style attribute + // unquoted, so an unescaped quote could break out of the attribute + // and inject markup. + const tooltip = renderNormalizedTooltip( + { ...params, color: 'red" onmouseover="alert(1)' }, + metrics, + mockGetDenormalizedValue, + metricsWithCustomBounds, + ); + expect(tooltip).not.toContain('" onmouseover="alert(1)"'); + expect(tooltip).toContain('" onmouseover="alert(1)'); + }); }); diff --git a/superset-frontend/plugins/plugin-chart-world-map/src/WorldMap.ts b/superset-frontend/plugins/plugin-chart-world-map/src/WorldMap.ts index 68d7cf61b92..afaaaac5719 100644 --- a/superset-frontend/plugins/plugin-chart-world-map/src/WorldMap.ts +++ b/superset-frontend/plugins/plugin-chart-world-map/src/WorldMap.ts @@ -80,6 +80,19 @@ interface DatamapSource { country?: string; } +/** + * Escape HTML special characters to prevent XSS attacks. Popup templates are + * assigned to the hover element via innerHTML by the datamaps library, and + * formatter output can echo a creator-controlled format string verbatim + * (see createD3NumberFormatter's invalid-format fallback), so both the name + * and the formatted value must be treated as untrusted text. + */ +function escapeHtml(text: string): string { + const div = document.createElement('div'); + div.textContent = text; + return div.innerHTML; +} + const propTypes = { data: PropTypes.arrayOf( PropTypes.shape({ @@ -279,9 +292,9 @@ function WorldMap(element: HTMLElement, props: WorldMapProps): void { highlightBorderWidth: 1, popupTemplate: (geo, d) => d && - `
${d.name}
${formatter( - d.m1, - )}
`, + `
${escapeHtml( + d.name, + )}
${escapeHtml(String(formatter(d.m1)))}
`, }, bubblesConfig: { borderWidth: 1, @@ -290,9 +303,9 @@ function WorldMap(element: HTMLElement, props: WorldMapProps): void { popupOnHover: !inContextMenu, radius: null, popupTemplate: (geo, d) => - `
${d.name}
${formatter( - d.m2, - )}
`, + `
${escapeHtml( + d.name, + )}
${escapeHtml(String(formatter(d.m2)))}
`, fillOpacity: 0.5, animate: true, highlightOnHover: !inContextMenu, diff --git a/superset-frontend/plugins/plugin-chart-world-map/test/WorldMap.test.ts b/superset-frontend/plugins/plugin-chart-world-map/test/WorldMap.test.ts index ac53e295eeb..96c30a1a4a9 100644 --- a/superset-frontend/plugins/plugin-chart-world-map/test/WorldMap.test.ts +++ b/superset-frontend/plugins/plugin-chart-world-map/test/WorldMap.test.ts @@ -180,6 +180,33 @@ test('disables Datamaps highlightOnHover while the context menu is open', () => expect(geographyConfig?.highlightOnHover).toBe(false); }); +test('escapes markup in hover popup templates', () => { + // Regression test for stored XSS via the number-formatter fallback: an + // invalid Y Axis Format string is echoed verbatim by the formatter + // (createD3NumberFormatter's catch branch), so the popup templates must + // HTML-escape formatter output before datamaps assigns it via innerHTML. + const maliciousFormatter = getNumberFormatter(''); + WorldMap(container, { ...baseProps, formatter: maliciousFormatter }); + + const geographyConfig = lastDatamapConfig?.geographyConfig as { + popupTemplate: (geo: unknown, d: unknown) => string; + }; + const bubblesConfig = lastDatamapConfig?.bubblesConfig as { + popupTemplate: (geo: unknown, d: unknown) => string; + }; + const entry = { name: 'United States', m1: 100, m2: 200 }; + + const geoPopup = geographyConfig.popupTemplate({}, entry); + const bubblePopup = bubblesConfig.popupTemplate({}, entry); + + [geoPopup, bubblePopup].forEach(popup => { + expect(popup).not.toContain(''); + expect(popup).toContain('<img src=x onerror=alert(1)>'); + expect(popup).toContain('<b>United States</b>'); + }); +}); + test('does not throw error when onContextMenu is undefined', () => { const propsWithoutContextMenu = { ...baseProps, diff --git a/superset-frontend/src/visualizations/TimeTable/components/LeftCell/LeftCell.test.tsx b/superset-frontend/src/visualizations/TimeTable/components/LeftCell/LeftCell.test.tsx index 5f21ad070c3..79169957835 100644 --- a/superset-frontend/src/visualizations/TimeTable/components/LeftCell/LeftCell.test.tsx +++ b/superset-frontend/src/visualizations/TimeTable/components/LeftCell/LeftCell.test.tsx @@ -140,4 +140,59 @@ describe('LeftCell', () => { 'http://example.com/sales?type=numeric&label=Sales Data', ); }); + + test('should not render javascript: URLs as links for column rows', () => { + const columnRow = { + label: 'Test Column', + column_name: 'test_column', + }; + + render( + , + ); + + expect(screen.queryByRole('link')).not.toBeInTheDocument(); + expect(screen.getByText('Test Column')).toBeInTheDocument(); + }); + + test('should not render script-bearing schemes assembled via templating', () => { + const columnRow = { + label: 'Test Column', + column_name: 'alert(1)', + }; + + render( + , + ); + + expect(screen.queryByRole('link')).not.toBeInTheDocument(); + }); + + test('should keep relative URLs as links', () => { + const columnRow = { + label: 'Test Column', + column_name: 'test_column', + }; + + render( + , + ); + + expect(screen.getByRole('link')).toHaveAttribute( + 'href', + '/superset/dashboard/test_column/', + ); + }); }); diff --git a/superset-frontend/src/visualizations/TimeTable/components/LeftCell/LeftCell.tsx b/superset-frontend/src/visualizations/TimeTable/components/LeftCell/LeftCell.tsx index b34a14e07c8..b628e31d7e8 100644 --- a/superset-frontend/src/visualizations/TimeTable/components/LeftCell/LeftCell.tsx +++ b/superset-frontend/src/visualizations/TimeTable/components/LeftCell/LeftCell.tsx @@ -28,6 +28,23 @@ interface LeftCellProps { url?: string; } +/** + * Confines a caller-supplied URL to http(s) and relative schemes before + * it's rendered as a link. Returns undefined for anything else, degrading + * the cell to plain text. + */ +export const toSafeHref = (url: string): string | undefined => { + try { + const { protocol } = new URL(url, window.location.origin); + if (protocol === 'http:' || protocol === 'https:') { + return url; + } + } catch { + // fall through: unparseable URLs are not rendered as links + } + return undefined; +}; + /** * Renders the left cell containing either column labels or metric information */ @@ -35,7 +52,7 @@ const LeftCell = ({ row, rowType, url }: LeftCellProps): ReactElement => { const fullUrl = useMemo(() => { if (!url) return undefined; const context = { metric: row }; - return Mustache.render(url, context); + return toSafeHref(Mustache.render(url, context)); }, [url, row]); if (rowType === 'column') {