You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/08/21 16:12:16 UTC

(superset) 03/06: fix(plugin-chart-echarts): escape series name, metric label, and value in Radar tooltip HTML

This is an automated email from the ASF dual-hosted git repository.

rusackas pushed a commit to branch fix/chart-tooltip-html-escaping
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 219a1d8921117b16b586c9e51309e99c5a69a687
Author: Superset Dev <de...@superset.apache.org>
AuthorDate: Fri Aug 21 01:24:04 2026 -0700

    fix(plugin-chart-echarts): escape series name, metric label, and value in Radar tooltip HTML
    
    renderNormalizedTooltip built its innerHTML tooltip string by interpolating
    seriesName (raw group-by values from query results), metric (creator
    labels), and value directly with no escaping, unlike every other tooltip
    path in this plugin (Rose, BoxPlot, Bullet, forecast.ts). Wrap all three
    interpolations in the plugin's existing sanitizeHtml helper.
---
 .../plugin-chart-echarts/src/Radar/utils.ts        | 17 ++++++++++++---
 .../plugin-chart-echarts/test/Radar/utils.test.ts  | 25 ++++++++++++++++++++++
 2 files changed, 39 insertions(+), 3 deletions(-)

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..f4358f0445a 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
@@ -85,19 +86,29 @@ export const renderNormalizedTooltip = (
     };
   });
 
+  // SECURITY: the tooltip is rendered via innerHTML (ECharts default
+  // renderMode 'html'). `seriesName` is built from raw group-by values in
+  // the query results and `metric` from creator-controlled metric labels,
+  // so both must be HTML-escaped — matching the sanitizeHtml/tooltipHtml
+  // treatment every other echarts tooltip path applies. Values are escaped
+  // too for consistency (formatter output is plain text).
   const tooltipRows = metricValues
     .map(
       ({ metric, value }) => `
         <div style="display:flex;">
-          <div>${colorDot}${metric}:</div>
-          <div style="font-weight:bold;margin-left:auto;">${value}</div>
+          <div>${colorDot}${sanitizeHtml(metric)}:</div>
+          <div style="font-weight:bold;margin-left:auto;">${sanitizeHtml(
+            String(value),
+          )}</div>
         </div>
       `,
     )
     .join('');
 
   return `
-    <div style="font-weight:bold;margin-bottom:5px;">${seriesName}</div>
+    <div style="font-weight:bold;margin-bottom:5px;">${sanitizeHtml(
+      seriesName,
+    )}</div>
     ${tooltipRows}
   `;
 };
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..db4bb4b2bee 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,29 @@ describe('renderNormalizedTooltip', () => {
     expect(tooltip).toContain('N/A');
     expect(tooltip).not.toContain('NaN');
   });
+
+  test('should HTML-escape series names from query data', () => {
+    // SECURITY regression test: the series name is built from raw group-by
+    // column values and the tooltip is rendered via innerHTML, so markup in
+    // warehouse data must not become live DOM.
+    const tooltip = renderNormalizedTooltip(
+      { ...params, name: '<img src=x onerror=alert(1)>' },
+      metrics,
+      mockGetDenormalizedValue,
+      metricsWithCustomBounds,
+    );
+    expect(tooltip).not.toContain('<img');
+    expect(tooltip).toContain('&lt;img');
+  });
+
+  test('should HTML-escape metric labels', () => {
+    const tooltip = renderNormalizedTooltip(
+      params,
+      ['<svg onload=alert(1)>', 'metric2'],
+      mockGetDenormalizedValue,
+      metricsWithCustomBounds,
+    );
+    expect(tooltip).not.toContain('<svg');
+    expect(tooltip).toContain('&lt;svg');
+  });
 });