Skip to content

Commit d8caeef

Browse files
committed
sec(frontend/history): escapeHtmlAttr on filter button label+column attrs
renderHistoryFilterButton injected `column` and `label` raw into aria-label, title, and data-column attributes via innerHTML template literal. Apply escapeHtmlAttr to both before interpolation and replace the em-dash in the active-state aria-label with a hyphen. Add history-filter-popover-xss.test.ts to assert hostile payloads in label and column are entity-encoded, not executed.
1 parent 9cf08c2 commit d8caeef

2 files changed

Lines changed: 69 additions & 2 deletions

File tree

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
/**
2+
* XSS regression for renderHistoryFilterButton (issue #166 follow-up).
3+
*
4+
* renderHistoryFilterButton injects `column` and `label` into HTML attribute
5+
* values (data-column, aria-label, title) via template literal. Any caller
6+
* passing a user-controlled string without escaping would produce a stored-XSS
7+
* vector. escapeHtmlAttr must be applied to both values before interpolation.
8+
*/
9+
10+
import { renderHistoryFilterButton } from '../lib/history-filter-popover';
11+
12+
// Use the real escapeHtmlAttr so DOM-based escaping is exercised, not a
13+
// pass-through stub.
14+
jest.mock('../lib/column-filters', () => ({
15+
parseNumericFilter: jest.fn(),
16+
}));
17+
18+
describe('renderHistoryFilterButton: attribute-injection XSS guard', () => {
19+
// This payload attempts to break out of the aria-label attribute and inject
20+
// a script element. In raw form it would produce:
21+
// aria-label=""><script>alert(1)</script><span x=""
22+
const SCRIPT_PAYLOAD = '"><script>alert(1)</script><span x="';
23+
// This payload injects an event handler attribute.
24+
const EVENT_PAYLOAD = '" onmouseover="alert(1)" x="';
25+
26+
test('hostile label does not inject raw <script> tag into attribute context', () => {
27+
const html = renderHistoryFilterButton('provider', SCRIPT_PAYLOAD, false);
28+
// The raw < > and " chars must be entity-encoded; no unescaped angle brackets.
29+
expect(html).not.toContain('"><script>');
30+
expect(html).not.toContain('</script>');
31+
// The encoded form is present (attribute value is escaped, not stripped).
32+
expect(html).toContain('&lt;script&gt;');
33+
});
34+
35+
test('hostile label does not inject event handler attribute', () => {
36+
const html = renderHistoryFilterButton('provider', EVENT_PAYLOAD, false);
37+
// The injected " must be encoded; no raw onmouseover= outside an attribute value.
38+
expect(html).not.toContain('" onmouseover=');
39+
// The encoded form must appear inside the attribute value.
40+
expect(html).toContain('&quot; onmouseover=');
41+
});
42+
43+
test('hostile column id does not inject raw markup into data-column', () => {
44+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
45+
const html = renderHistoryFilterButton(SCRIPT_PAYLOAD as any, 'Provider', false);
46+
expect(html).not.toContain('><script>');
47+
expect(html).not.toContain('</script>');
48+
expect(html).toContain('&lt;script&gt;');
49+
});
50+
51+
test('safe column and label pass through correctly', () => {
52+
const html = renderHistoryFilterButton('provider', 'Provider', false);
53+
expect(html).toContain('data-column="provider"');
54+
expect(html).toContain('aria-label="Filter Provider"');
55+
expect(html).toContain('class="history-column-filter-btn"');
56+
});
57+
58+
test('active flag adds "active" class and updates aria-label', () => {
59+
const html = renderHistoryFilterButton('savings', 'Monthly Savings', true);
60+
expect(html).toContain('class="history-column-filter-btn active"');
61+
expect(html).toContain('aria-label="Filter Monthly Savings - currently active"');
62+
});
63+
});

‎frontend/src/lib/history-filter-popover.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
parseNumericFilter,
2020
type ColumnFilterKind,
2121
} from './column-filters';
22+
import { escapeHtmlAttr } from '../utils';
2223

2324
export interface PopoverConfig<TColumnId extends string> {
2425
column: TColumnId;
@@ -350,8 +351,11 @@ export function renderHistoryFilterButton<TColumnId extends string>(
350351
active: boolean,
351352
): string {
352353
const cls = `history-column-filter-btn${active ? ' active' : ''}`;
353-
const ariaLabel = active ? `Filter ${label} — currently active` : `Filter ${label}`;
354+
const ariaLabel = escapeHtmlAttr(
355+
active ? `Filter ${label} - currently active` : `Filter ${label}`,
356+
);
357+
const safeColumn = escapeHtmlAttr(String(column));
354358
// Use the same gear-style glyph (⛛) the recommendations popover
355359
// uses so the two surfaces are visually identical to the user.
356-
return `<button type="button" class="${cls}" data-column="${column}" aria-haspopup="dialog" aria-expanded="false" aria-label="${ariaLabel}" title="${ariaLabel}">⛛</button>`;
360+
return `<button type="button" class="${cls}" data-column="${safeColumn}" aria-haspopup="dialog" aria-expanded="false" aria-label="${ariaLabel}" title="${ariaLabel}">⛛</button>`;
357361
}

0 commit comments

Comments
 (0)