Skip to content

Commit 9c6a002

Browse files
authored
fix: move innerHTML as separate assignment to improve CSP trusted types (#1162)
* fix: move `innerHTML` as separate assignment outside of createDomElement - to further improve CSP support (Content Security Policy), we need to move `innerHTML` as separate assignment and not use it directly within a `createDomElement`, so for example this line `const elm = createDomElement('div', { innerHTML: '' })` should be split in 2 lines `const elm = createDomElement('div'); elm.innerHTML = '';` * chore: add `RETURN_TRUSTED_TYPE: true` to improve CSP
1 parent f7b8c46 commit 9c6a002

File tree

9 files changed

+49
-33
lines changed

9 files changed

+49
-33
lines changed

packages/common/src/editors/autocompleterEditor.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -512,7 +512,9 @@ export class AutocompleterEditor<T extends AutocompleteItem = any> implements Ed
512512
// for the remaining allowed tags we'll permit all attributes
513513
const sanitizedTemplateText = sanitizeTextByAvailableSanitizer(this.gridOptions, templateString) || '';
514514

515-
return createDomElement('div', { innerHTML: sanitizedTemplateText });
515+
const tmpElm = document.createElement('div');
516+
tmpElm.innerHTML = sanitizedTemplateText;
517+
return tmpElm;
516518
}
517519

518520
protected renderCollectionItem(item: any) { // CollectionCustomStructure

packages/common/src/extensions/extensionCommonUtils.ts

+4-6
Original file line numberDiff line numberDiff line change
@@ -141,12 +141,10 @@ export function populateColumnPicker(this: SlickColumnPicker | SlickGridMenu, ad
141141
const headerColumnValueExtractorFn = typeof addonOptions?.headerColumnValueExtractor === 'function' ? addonOptions.headerColumnValueExtractor : context._defaults.headerColumnValueExtractor;
142142
const columnLabel = headerColumnValueExtractorFn!(column, context.gridOptions);
143143

144-
columnLiElm.appendChild(
145-
createDomElement('label', {
146-
htmlFor: `${context._gridUid}-${menuPrefix}colpicker-${columnId}`,
147-
innerHTML: sanitizeTextByAvailableSanitizer(context.gridOptions, columnLabel),
148-
})
149-
);
144+
const labelElm = document.createElement('label');
145+
labelElm.htmlFor = `${context._gridUid}-${menuPrefix}colpicker-${columnId}`;
146+
labelElm.innerHTML = sanitizeTextByAvailableSanitizer(context.gridOptions, columnLabel);
147+
columnLiElm.appendChild(labelElm);
150148
context._listElm.appendChild(columnLiElm);
151149
}
152150

packages/common/src/filters/autocompleterFilter.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -572,9 +572,9 @@ export class AutocompleterFilter<T extends AutocompleteItem = any> implements Fi
572572
// for the remaining allowed tags we'll permit all attributes
573573
const sanitizedTemplateText = sanitizeTextByAvailableSanitizer(this.gridOptions, templateString) || '';
574574

575-
return createDomElement('div', {
576-
innerHTML: sanitizedTemplateText
577-
});
575+
const tmpDiv = document.createElement('div');
576+
tmpDiv.innerHTML = sanitizedTemplateText;
577+
return tmpDiv;
578578
}
579579

580580
protected renderCollectionItem(item: any) {

packages/common/src/filters/filterUtilities.ts

+4-6
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,10 @@ export function buildSelectOperator(optionValues: Array<{ operator: OperatorStri
1515
const selectElm = createDomElement('select', { className: 'form-control' });
1616

1717
for (const option of optionValues) {
18-
selectElm.appendChild(
19-
createDomElement('option', {
20-
value: option.operator,
21-
innerHTML: sanitizeTextByAvailableSanitizer(gridOptions, `${htmlEncodedStringWithPadding(option.operator, 3)}${option.description}`)
22-
})
23-
);
18+
const optionElm = document.createElement('option');
19+
optionElm.value = option.operator;
20+
optionElm.innerHTML = sanitizeTextByAvailableSanitizer(gridOptions, `${htmlEncodedStringWithPadding(option.operator, 3)}${option.description}`);
21+
selectElm.appendChild(optionElm);
2422
}
2523

2624
return selectElm;

packages/common/src/services/__tests__/domUtilities.spec.ts

+20-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import 'jest-extended';
22
import { GridOption } from '../../interfaces';
33
import {
44
calculateAvailableSpace,
5+
createDomElement,
56
emptyElement,
67
findFirstElementAttribute,
78
getElementOffsetRelativeToParent,
@@ -46,6 +47,24 @@ describe('Service/domUtilies', () => {
4647
});
4748
});
4849

50+
describe('createDomElement method', () => {
51+
it('should create a DOM element via the method to equal a regular DOM element', () => {
52+
const div = document.createElement('div');
53+
div.className = 'red bold';
54+
const cdiv = createDomElement('div', { className: 'red bold' });
55+
56+
expect(cdiv).toEqual(div);
57+
expect(cdiv.outerHTML).toEqual(div.outerHTML);
58+
});
59+
60+
it('should display a warning when trying to use innerHTML via the method', () => {
61+
const consoleWarnSpy = jest.spyOn(global.console, 'warn').mockReturnValue();
62+
createDomElement('div', { className: 'red bold', innerHTML: '<input />' });
63+
64+
expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining(`[Slickgrid-Universal] For better CSP (Content Security Policy) support, do not use "innerHTML" directly in "createDomElement('div', { innerHTML: 'some html'})"`));
65+
});
66+
});
67+
4968
describe('emptyElement method', () => {
5069
const div = document.createElement('div');
5170
div.innerHTML = `<ul><li>Item 1</li><li>Item 2</li></ul>`;
@@ -112,7 +131,7 @@ describe('Service/domUtilies', () => {
112131
document.body.appendChild(div);
113132

114133
it('should return undefined when element if not a valid html element', () => {
115-
const output = getHtmlElementOffset(null);
134+
const output = getHtmlElementOffset(null as any);
116135
expect(output).toEqual(undefined);
117136
});
118137

packages/common/src/services/domUtilities.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,10 @@ export function createDomElement<T extends keyof HTMLElementTagNameMap, K extend
161161

162162
if (elementOptions) {
163163
Object.keys(elementOptions).forEach((elmOptionKey) => {
164+
if (elmOptionKey === 'innerHTML') {
165+
console.warn(`[Slickgrid-Universal] For better CSP (Content Security Policy) support, do not use "innerHTML" directly in "createDomElement('${tagName}', { innerHTML: 'some html'})", ` +
166+
`it is better as separate assignment: "const elm = createDomElement('span'); elm.innerHTML = 'some html';"`);
167+
}
164168
const elmValue = elementOptions[elmOptionKey as keyof typeof elementOptions];
165169
if (typeof elmValue === 'object') {
166170
Object.assign(elm[elmOptionKey as K] as object, elmValue);
@@ -362,7 +366,7 @@ export function sanitizeTextByAvailableSanitizer(gridOptions: GridOption, dirtyH
362366
if (typeof gridOptions?.sanitizer === 'function') {
363367
sanitizedText = gridOptions.sanitizer(dirtyHtml || '');
364368
} else if (typeof DOMPurify?.sanitize === 'function') {
365-
sanitizedText = (DOMPurify.sanitize(dirtyHtml || '', domPurifyOptions || {}) || '').toString();
369+
sanitizedText = (DOMPurify.sanitize(dirtyHtml || '', domPurifyOptions || { RETURN_TRUSTED_TYPE: true }) || '').toString();
366370
}
367371

368372
return sanitizedText;

packages/composite-editor-component/src/slick-composite-editor.component.ts

+5-8
Original file line numberDiff line numberDiff line change
@@ -371,10 +371,9 @@ export class SlickCompositeEditorComponent implements ExternalResource {
371371
modalContentElm.classList.add(splitClassName);
372372
}
373373

374-
const modalHeaderTitleElm = createDomElement('div', {
375-
className: 'slick-editor-modal-title',
376-
innerHTML: sanitizeTextByAvailableSanitizer(this.gridOptions, parsedHeaderTitle),
377-
});
374+
const modalHeaderTitleElm = createDomElement('div', { className: 'slick-editor-modal-title' });
375+
modalHeaderTitleElm.innerHTML = sanitizeTextByAvailableSanitizer(this.gridOptions, parsedHeaderTitle);
376+
378377
const modalCloseButtonElm = createDomElement('button', { type: 'button', ariaLabel: 'Close', textContent: '×', className: 'close', dataset: { action: 'close' } });
379378
if (this._options.showCloseButtonOutside) {
380379
modalHeaderTitleElm?.classList?.add('outside');
@@ -451,10 +450,8 @@ export class SlickCompositeEditorComponent implements ExternalResource {
451450
itemContainer.classList.add('slick-col-medium-6', `slick-col-xlarge-${12 / layoutColCount}`);
452451
}
453452

454-
const templateItemLabelElm = createDomElement('div', {
455-
className: `item-details-label editor-${columnDef.id}`,
456-
innerHTML: sanitizeTextByAvailableSanitizer(this.gridOptions, this.getColumnLabel(columnDef) || 'n/a')
457-
});
453+
const templateItemLabelElm = createDomElement('div', { className: `item-details-label editor-${columnDef.id}` });
454+
templateItemLabelElm.innerHTML = sanitizeTextByAvailableSanitizer(this.gridOptions, this.getColumnLabel(columnDef) || 'n/a');
458455
const templateItemEditorElm = createDomElement('div', {
459456
className: 'item-details-editor-container slick-cell',
460457
dataset: { editorid: `${columnDef.id}` },

packages/custom-footer-component/src/slick-footer.component.ts

+3-6
Original file line numberDiff line numberDiff line change
@@ -176,12 +176,9 @@ export class SlickFooterComponent {
176176
}
177177
});
178178

179-
footerElm.appendChild(
180-
createDomElement('div', {
181-
className: `left-footer ${this.customFooterOptions.leftContainerClass}`,
182-
innerHTML: sanitizeTextByAvailableSanitizer(this.gridOptions, this.customFooterOptions.leftFooterText || '')
183-
})
184-
);
179+
const leftFooterElm = createDomElement('div', { className: `left-footer ${this.customFooterOptions.leftContainerClass}` });
180+
leftFooterElm.innerHTML = sanitizeTextByAvailableSanitizer(this.gridOptions, this.customFooterOptions.leftFooterText || '');
181+
footerElm.appendChild(leftFooterElm);
185182
footerElm.appendChild(this.createFooterRightContainer());
186183
this._footerElement = footerElm;
187184

packages/custom-tooltip-plugin/src/slickCustomTooltip.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,8 @@ export class SlickCustomTooltip {
316316
* also clear the "title" attribute from the grid div text content so that it won't show also as a 2nd browser tooltip
317317
*/
318318
protected renderRegularTooltip(formatterOrText: Formatter | string | undefined, cell: { row: number; cell: number; }, value: any, columnDef: Column, item: any) {
319-
const tmpDiv = createDomElement('div', { innerHTML: this.parseFormatterAndSanitize(formatterOrText, cell, value, columnDef, item) });
319+
const tmpDiv = document.createElement('div');
320+
tmpDiv.innerHTML = this.parseFormatterAndSanitize(formatterOrText, cell, value, columnDef, item);
320321

321322
let tooltipText = columnDef?.toolTip ?? '';
322323
let tmpTitleElm: HTMLDivElement | null | undefined;

0 commit comments

Comments
 (0)