fix(security): escape cell values in status renderers to prevent DOM XSS
The error/no-spinner/spinner cell renderers wrote the cell value straight into td.innerHTML. A value containing markup (which can arrive from a dataset served by the getdata stored program or from a typed edit) was therefore parsed and executed by the browser. Escape the value so it renders as inert text, keeping the hardcoded icon markup intact, and add a regression test that reproduces the injection via a real Handsontable instance.
This commit is contained in:
@@ -1,5 +1,10 @@
|
||||
import Handsontable from 'handsontable'
|
||||
import { makeNumberFormatRenderer } from './renderers.utils'
|
||||
import {
|
||||
makeNumberFormatRenderer,
|
||||
errorRenderer,
|
||||
noSpinnerRenderer,
|
||||
spinnerRenderer
|
||||
} from './renderers.utils'
|
||||
|
||||
describe('makeNumberFormatRenderer', () => {
|
||||
it('renders a numeric cell as EUR currency without changing the value', () => {
|
||||
@@ -86,3 +91,67 @@ describe('makeNumberFormatRenderer', () => {
|
||||
container.remove()
|
||||
})
|
||||
})
|
||||
|
||||
/**
|
||||
* DOM-injection reproduction mirroring the editor's cell-render cycle.
|
||||
* During dynamic cell validation the editor applies one of the status
|
||||
* renderers to a cell via setCellMeta + hot.render(). Those renderers paint
|
||||
* the cell value with td.innerHTML, so a value containing markup is injected
|
||||
* and executed (the <img onerror> fires in the browser). The value can come
|
||||
* straight from a dataset row served by the getdata stored program, or from a
|
||||
* typed edit. These fail on the vulnerable implementation and pass once the
|
||||
* renderer escapes the value.
|
||||
*/
|
||||
describe('grid cell renderers do not inject raw HTML', () => {
|
||||
const maliciousValue = '<img src=x onerror=alert(1)>'
|
||||
|
||||
// Seed a real Handsontable grid with the payload as a loaded cell value,
|
||||
// then apply the given status renderer and render — exactly the sequence the
|
||||
// editor uses during the dynamic-validation cycle.
|
||||
const renderWith = (
|
||||
renderer: (
|
||||
i: any,
|
||||
td: any,
|
||||
r: number,
|
||||
c: number,
|
||||
p: any,
|
||||
v: any,
|
||||
cp: any
|
||||
) => any
|
||||
) => {
|
||||
const container = document.createElement('div')
|
||||
document.body.appendChild(container)
|
||||
const hot = new Handsontable(container, {
|
||||
data: [{ SOME_CHAR: maliciousValue }],
|
||||
columns: [{ data: 'SOME_CHAR', type: 'text' }],
|
||||
licenseKey: 'non-commercial-and-evaluation'
|
||||
})
|
||||
hot.render()
|
||||
hot.setCellMeta(0, 0, 'renderer', renderer)
|
||||
hot.render()
|
||||
const td: HTMLTableCellElement | null = hot.getCell(0, 0)
|
||||
hot.destroy()
|
||||
container.remove()
|
||||
return td
|
||||
}
|
||||
|
||||
// A vulnerable renderer turns the value into a real <img> element with an
|
||||
// onerror handler (proven by the browser firing alert(1)). A safe
|
||||
// renderer leaves no such element. Asserting on the parsed DOM rather
|
||||
// than the raw string avoids false passes from browser attribute normalising.
|
||||
const assertNoInjectedElement = (td: HTMLTableCellElement | null) => {
|
||||
expect(td?.querySelector('img[onerror]')).toBeNull()
|
||||
}
|
||||
|
||||
it('noSpinnerRenderer escapes rather than injecting the value', () => {
|
||||
assertNoInjectedElement(renderWith(noSpinnerRenderer))
|
||||
})
|
||||
|
||||
it('errorRenderer escapes rather than injecting the value', () => {
|
||||
assertNoInjectedElement(renderWith(errorRenderer))
|
||||
})
|
||||
|
||||
it('spinnerRenderer escapes rather than injecting the value', () => {
|
||||
assertNoInjectedElement(renderWith(spinnerRenderer))
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,5 +1,23 @@
|
||||
import Handsontable from 'handsontable'
|
||||
|
||||
/**
|
||||
* Returns string-safe text of any value so it can be assigned to innerHTML.
|
||||
* The cell values painted by the status renderers are user/DB-controlled,
|
||||
* so they must never be parsed as HTML by the browser — escaping turns any
|
||||
* embedded markup into inert text.
|
||||
*/
|
||||
const escapeHtml = (value: any): string =>
|
||||
String(value ?? '').replace(/[&<>"']/g, (char) => {
|
||||
const entities: Record<string, string> = {
|
||||
'&': '&',
|
||||
'<': '<',
|
||||
'>': '>',
|
||||
'"': '"',
|
||||
"'": '''
|
||||
}
|
||||
return entities[char]
|
||||
})
|
||||
|
||||
/**
|
||||
* Builds a display-only HOT renderer that formats numeric cell values using
|
||||
* Intl.NumberFormat. The stored/submitted value is never changed — only the
|
||||
@@ -67,9 +85,7 @@ export const errorRenderer = (
|
||||
) => {
|
||||
addDarkClass(td)
|
||||
|
||||
td.innerHTML = `${
|
||||
value ? value.toString() : ''
|
||||
} <cds-icon shape="exclamation-triangle" status="warning"></cds-icon>`
|
||||
td.innerHTML = `${escapeHtml(value)} <cds-icon shape="exclamation-triangle" status="warning"></cds-icon>`
|
||||
|
||||
return td
|
||||
}
|
||||
@@ -89,7 +105,7 @@ export const noSpinnerRenderer = (
|
||||
) => {
|
||||
addDarkClass(td)
|
||||
|
||||
td.innerHTML = value ? value : ''
|
||||
td.innerHTML = escapeHtml(value)
|
||||
|
||||
return td
|
||||
}
|
||||
@@ -110,9 +126,7 @@ export const spinnerRenderer = (
|
||||
) => {
|
||||
addDarkClass(td)
|
||||
|
||||
td.innerHTML = `${
|
||||
value ? value.toString() : ''
|
||||
} <span class="spinner spinner-sm vertical-align-middle"></span>`
|
||||
td.innerHTML = `${escapeHtml(value)} <span class="spinner spinner-sm vertical-align-middle"></span>`
|
||||
|
||||
return td
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user