Skip to content

Commit d73ff25

Browse files
authored
fix(appkit-ui): escape heatmap tooltip values to prevent XSS (#437)
* fix(appkit-ui): escape heatmap tooltip values to prevent XSS The heatmap tooltip used an ECharts function formatter that returned raw interpolated data values. Unlike string-template formatters, function formatter return values are injected as raw HTML into the tooltip DOM, so a tenant-supplied category value like <img src=x onerror=...> would execute on hover. Add an escapeHtml helper to charts/utils.ts and escape the x/y labels and value in the heatmap tooltip formatter. Add regression tests asserting the formatter output contains no raw HTML. Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com> * test(appkit-ui): add escapeHtml unit coverage and usage caveat - Add focused escapeHtml unit tests: all five escaped characters, ampersand-first ordering, and expected double-escaping of pre-escaped input. - Add JSDoc caveat: escapeHtml is only for HTML tooltip contexts, not canvas-rendered axis/series label formatters. Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com> --------- Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com> Co-authored-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
1 parent c7d93d2 commit d73ff25

4 files changed

Lines changed: 91 additions & 4 deletions

File tree

packages/appkit-ui/src/react/charts/__tests__/options.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ import {
1212
interface EChartsOption {
1313
title?: { text?: string };
1414
legend?: unknown;
15+
tooltip?: {
16+
formatter?: (params: { data: [number, number, number] }) => string;
17+
};
1518
xAxis: { type: string; data?: unknown[] };
1619
yAxis: { type: string; data?: unknown[] };
1720
series: Array<{
@@ -646,4 +649,28 @@ describe("buildHeatmapOption", () => {
646649

647650
expect(opt.series[0].data).toEqual(ctx.heatmapData);
648651
});
652+
653+
test("tooltip formatter renders labels and values", () => {
654+
const ctx = createHeatmapContext();
655+
const opt = asOption(buildHeatmapOption(ctx));
656+
657+
const output = opt.tooltip?.formatter?.({ data: [1, 2, 25] });
658+
expect(output).toBe("10AM, Wed: 25");
659+
});
660+
661+
test("tooltip formatter escapes HTML in category values (XSS)", () => {
662+
const ctx = {
663+
...createHeatmapContext(),
664+
xData: ["<img src=x onerror=alert(1)>", "10AM"],
665+
yAxisData: ["<script>alert(2)</script>", "Tue"],
666+
};
667+
const opt = asOption(buildHeatmapOption(ctx));
668+
669+
const output = opt.tooltip?.formatter?.({ data: [0, 0, 10] });
670+
expect(output).not.toContain("<");
671+
expect(output).not.toContain(">");
672+
expect(output).toBe(
673+
"&lt;img src=x onerror=alert(1)&gt;, &lt;script&gt;alert(2)&lt;/script&gt;: 10",
674+
);
675+
});
649676
});

packages/appkit-ui/src/react/charts/__tests__/utils.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { describe, expect, test } from "vitest";
22
import {
33
createTimeSeriesData,
4+
escapeHtml,
45
formatLabel,
56
sortTimeSeriesAscending,
67
toChartArray,
@@ -129,6 +130,39 @@ describe("formatLabel", () => {
129130
});
130131
});
131132

133+
describe("escapeHtml", () => {
134+
test("escapes all five HTML special characters", () => {
135+
expect(escapeHtml("&")).toBe("&amp;");
136+
expect(escapeHtml("<")).toBe("&lt;");
137+
expect(escapeHtml(">")).toBe("&gt;");
138+
expect(escapeHtml('"')).toBe("&quot;");
139+
expect(escapeHtml("'")).toBe("&#39;");
140+
});
141+
142+
test("escapes all special characters in a combined string", () => {
143+
expect(escapeHtml(`<img src="x" onerror='alert(1)'>&`)).toBe(
144+
"&lt;img src=&quot;x&quot; onerror=&#39;alert(1)&#39;&gt;&amp;",
145+
);
146+
});
147+
148+
test("escapes ampersand first so generated entities are not double-encoded", () => {
149+
// "&" is replaced before "<" and ">", so the entities produced by
150+
// escaping "<" and ">" keep their single "&" prefix.
151+
expect(escapeHtml("<&>")).toBe("&lt;&amp;&gt;");
152+
});
153+
154+
test("double-escapes pre-escaped input (expected for raw data)", () => {
155+
// escapeHtml assumes raw, unescaped input; already-escaped entities
156+
// are escaped again. This is intentional for untrusted data.
157+
expect(escapeHtml("&amp;")).toBe("&amp;amp;");
158+
});
159+
160+
test("leaves safe strings unchanged", () => {
161+
expect(escapeHtml("hello world 123")).toBe("hello world 123");
162+
expect(escapeHtml("")).toBe("");
163+
});
164+
});
165+
132166
describe("truncateLabel", () => {
133167
test("truncates long strings with ellipsis", () => {
134168
// Implementation: value.slice(0, maxLength) + "..."

packages/appkit-ui/src/react/charts/options.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
import type { ChartType } from "./types";
2-
import { createTimeSeriesData, formatLabel, truncateLabel } from "./utils";
2+
import {
3+
createTimeSeriesData,
4+
escapeHtml,
5+
formatLabel,
6+
truncateLabel,
7+
} from "./utils";
38

49
// ============================================================================
510
// Option Builder Types
@@ -189,9 +194,11 @@ export function buildHeatmapOption(
189194
trigger: "item",
190195
formatter: (params: { data: [number, number, number] }) => {
191196
const [xIdx, yIdx, value] = params.data;
192-
const xLabel = ctx.xData[xIdx] ?? xIdx;
193-
const yLabel = ctx.yAxisData[yIdx] ?? yIdx;
194-
return `${xLabel}, ${yLabel}: ${value}`;
197+
// Function formatter output is injected as raw HTML into the
198+
// tooltip DOM, so data-derived labels must be escaped.
199+
const xLabel = escapeHtml(String(ctx.xData[xIdx] ?? xIdx));
200+
const yLabel = escapeHtml(String(ctx.yAxisData[yIdx] ?? yIdx));
201+
return `${xLabel}, ${yLabel}: ${escapeHtml(String(value))}`;
195202
},
196203
},
197204
grid: {

packages/appkit-ui/src/react/charts/utils.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,25 @@ export function formatLabel(field: string): string {
5555
);
5656
}
5757

58+
/**
59+
* Escapes HTML special characters to prevent XSS.
60+
* Required for ECharts function formatters: unlike string-template
61+
* formatters, their return values are injected as raw HTML into the
62+
* tooltip DOM.
63+
*
64+
* Only for HTML tooltip contexts; do NOT use on canvas-rendered
65+
* axis/series label formatters — canvas text is not HTML and would
66+
* display literal entities (e.g. "&amp;").
67+
*/
68+
export function escapeHtml(value: string): string {
69+
return value
70+
.replace(/&/g, "&amp;")
71+
.replace(/</g, "&lt;")
72+
.replace(/>/g, "&gt;")
73+
.replace(/"/g, "&quot;")
74+
.replace(/'/g, "&#39;");
75+
}
76+
5877
/**
5978
* Truncates a label to a maximum length with ellipsis.
6079
*/

0 commit comments

Comments
 (0)