fix: move innerHTML as separate assignment to improve CSP trusted t…

…ypes (#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
ghiscoding · Nov 1, 2023 · 9c6a002 · 9c6a002
1 parent f7b8c46
commit 9c6a002
Show file tree

Hide file tree

Showing 9 changed files with 49 additions and 33 deletions.
diff --git a/packages/common/src/editors/autocompleterEditor.ts b/packages/common/src/editors/autocompleterEditor.ts
@@ -512,7 +512,9 @@ export class AutocompleterEditor<T extends AutocompleteItem = any> implements Ed
     // for the remaining allowed tags we'll permit all attributes
     const sanitizedTemplateText = sanitizeTextByAvailableSanitizer(this.gridOptions, templateString) || '';
 
-    return createDomElement('div', { innerHTML: sanitizedTemplateText });
+    const tmpElm = document.createElement('div');
+    tmpElm.innerHTML = sanitizedTemplateText;
+    return tmpElm;
   }
 
   protected renderCollectionItem(item: any) { // CollectionCustomStructure

diff --git a/packages/common/src/extensions/extensionCommonUtils.ts b/packages/common/src/extensions/extensionCommonUtils.ts
@@ -141,12 +141,10 @@ export function populateColumnPicker(this: SlickColumnPicker | SlickGridMenu, ad
     const headerColumnValueExtractorFn = typeof addonOptions?.headerColumnValueExtractor === 'function' ? addonOptions.headerColumnValueExtractor : context._defaults.headerColumnValueExtractor;
     const columnLabel = headerColumnValueExtractorFn!(column, context.gridOptions);
 
-    columnLiElm.appendChild(
-      createDomElement('label', {
-        htmlFor: `${context._gridUid}-${menuPrefix}colpicker-${columnId}`,
-        innerHTML: sanitizeTextByAvailableSanitizer(context.gridOptions, columnLabel),
-      })
-    );
+    const labelElm = document.createElement('label');
+    labelElm.htmlFor = `${context._gridUid}-${menuPrefix}colpicker-${columnId}`;
+    labelElm.innerHTML = sanitizeTextByAvailableSanitizer(context.gridOptions, columnLabel);
+    columnLiElm.appendChild(labelElm);
     context._listElm.appendChild(columnLiElm);
   }
 

diff --git a/packages/common/src/filters/autocompleterFilter.ts b/packages/common/src/filters/autocompleterFilter.ts
@@ -572,9 +572,9 @@ export class AutocompleterFilter<T extends AutocompleteItem = any> implements Fi
     // for the remaining allowed tags we'll permit all attributes
     const sanitizedTemplateText = sanitizeTextByAvailableSanitizer(this.gridOptions, templateString) || '';
 
-    return createDomElement('div', {
-      innerHTML: sanitizedTemplateText
-    });
+    const tmpDiv = document.createElement('div');
+    tmpDiv.innerHTML = sanitizedTemplateText;
+    return tmpDiv;
   }
 
   protected renderCollectionItem(item: any) {

diff --git a/packages/common/src/filters/filterUtilities.ts b/packages/common/src/filters/filterUtilities.ts
@@ -15,12 +15,10 @@ export function buildSelectOperator(optionValues: Array<{ operator: OperatorStri
   const selectElm = createDomElement('select', { className: 'form-control' });
 
   for (const option of optionValues) {
-    selectElm.appendChild(
-      createDomElement('option', {
-        value: option.operator,
-        innerHTML: sanitizeTextByAvailableSanitizer(gridOptions, `${htmlEncodedStringWithPadding(option.operator, 3)}${option.description}`)
-      })
-    );
+    const optionElm = document.createElement('option');
+    optionElm.value = option.operator;
+    optionElm.innerHTML = sanitizeTextByAvailableSanitizer(gridOptions, `${htmlEncodedStringWithPadding(option.operator, 3)}${option.description}`);
+    selectElm.appendChild(optionElm);
   }
 
   return selectElm;

diff --git a/packages/common/src/services/__tests__/domUtilities.spec.ts b/packages/common/src/services/__tests__/domUtilities.spec.ts
@@ -2,6 +2,7 @@ import 'jest-extended';
 import { GridOption } from '../../interfaces';
 import {
   calculateAvailableSpace,
+  createDomElement,
   emptyElement,
   findFirstElementAttribute,
   getElementOffsetRelativeToParent,
@@ -46,6 +47,24 @@ describe('Service/domUtilies', () => {
     });
   });
 
+  describe('createDomElement method', () => {
+    it('should create a DOM element via the method to equal a regular DOM element', () => {
+      const div = document.createElement('div');
+      div.className = 'red bold';
+      const cdiv = createDomElement('div', { className: 'red bold' });
+
+      expect(cdiv).toEqual(div);
+      expect(cdiv.outerHTML).toEqual(div.outerHTML);
+    });
+
+    it('should display a warning when trying to use innerHTML via the method', () => {
+      const consoleWarnSpy = jest.spyOn(global.console, 'warn').mockReturnValue();
+      createDomElement('div', { className: 'red bold', innerHTML: '<input />' });
+
+      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'})"`));
+    });
+  });
+
   describe('emptyElement method', () => {
     const div = document.createElement('div');
     div.innerHTML = `<ul><li>Item 1</li><li>Item 2</li></ul>`;
@@ -112,7 +131,7 @@ describe('Service/domUtilies', () => {
     document.body.appendChild(div);
 
     it('should return undefined when element if not a valid html element', () => {
-      const output = getHtmlElementOffset(null);
+      const output = getHtmlElementOffset(null as any);
       expect(output).toEqual(undefined);
     });
 

diff --git a/packages/common/src/services/domUtilities.ts b/packages/common/src/services/domUtilities.ts
@@ -161,6 +161,10 @@ export function createDomElement<T extends keyof HTMLElementTagNameMap, K extend
 
   if (elementOptions) {
     Object.keys(elementOptions).forEach((elmOptionKey) => {
+      if (elmOptionKey === 'innerHTML') {
+        console.warn(`[Slickgrid-Universal] For better CSP (Content Security Policy) support, do not use "innerHTML" directly in "createDomElement('${tagName}', { innerHTML: 'some html'})", ` +
+          `it is better as separate assignment: "const elm = createDomElement('span'); elm.innerHTML = 'some html';"`);
+      }
       const elmValue = elementOptions[elmOptionKey as keyof typeof elementOptions];
       if (typeof elmValue === 'object') {
         Object.assign(elm[elmOptionKey as K] as object, elmValue);
@@ -362,7 +366,7 @@ export function sanitizeTextByAvailableSanitizer(gridOptions: GridOption, dirtyH
   if (typeof gridOptions?.sanitizer === 'function') {
     sanitizedText = gridOptions.sanitizer(dirtyHtml || '');
   } else if (typeof DOMPurify?.sanitize === 'function') {
-    sanitizedText = (DOMPurify.sanitize(dirtyHtml || '', domPurifyOptions || {}) || '').toString();
+    sanitizedText = (DOMPurify.sanitize(dirtyHtml || '', domPurifyOptions || { RETURN_TRUSTED_TYPE: true }) || '').toString();
   }
 
   return sanitizedText;

diff --git a/packages/composite-editor-component/src/slick-composite-editor.component.ts b/packages/composite-editor-component/src/slick-composite-editor.component.ts
@@ -371,10 +371,9 @@ export class SlickCompositeEditorComponent implements ExternalResource {
           modalContentElm.classList.add(splitClassName);
         }
 
-        const modalHeaderTitleElm = createDomElement('div', {
-          className: 'slick-editor-modal-title',
-          innerHTML: sanitizeTextByAvailableSanitizer(this.gridOptions, parsedHeaderTitle),
-        });
+        const modalHeaderTitleElm = createDomElement('div', { className: 'slick-editor-modal-title' });
+        modalHeaderTitleElm.innerHTML = sanitizeTextByAvailableSanitizer(this.gridOptions, parsedHeaderTitle);
+
         const modalCloseButtonElm = createDomElement('button', { type: 'button', ariaLabel: 'Close', textContent: '×', className: 'close', dataset: { action: 'close' } });
         if (this._options.showCloseButtonOutside) {
           modalHeaderTitleElm?.classList?.add('outside');
@@ -451,10 +450,8 @@ export class SlickCompositeEditorComponent implements ExternalResource {
               itemContainer.classList.add('slick-col-medium-6', `slick-col-xlarge-${12 / layoutColCount}`);
             }
 
-            const templateItemLabelElm = createDomElement('div', {
-              className: `item-details-label editor-${columnDef.id}`,
-              innerHTML: sanitizeTextByAvailableSanitizer(this.gridOptions, this.getColumnLabel(columnDef) || 'n/a')
-            });
+            const templateItemLabelElm = createDomElement('div', { className: `item-details-label editor-${columnDef.id}` });
+            templateItemLabelElm.innerHTML = sanitizeTextByAvailableSanitizer(this.gridOptions, this.getColumnLabel(columnDef) || 'n/a');
             const templateItemEditorElm = createDomElement('div', {
               className: 'item-details-editor-container slick-cell',
               dataset: { editorid: `${columnDef.id}` },

diff --git a/packages/custom-footer-component/src/slick-footer.component.ts b/packages/custom-footer-component/src/slick-footer.component.ts
@@ -176,12 +176,9 @@ export class SlickFooterComponent {
       }
     });
 
-    footerElm.appendChild(
-      createDomElement('div', {
-        className: `left-footer ${this.customFooterOptions.leftContainerClass}`,
-        innerHTML: sanitizeTextByAvailableSanitizer(this.gridOptions, this.customFooterOptions.leftFooterText || '')
-      })
-    );
+    const leftFooterElm = createDomElement('div', { className: `left-footer ${this.customFooterOptions.leftContainerClass}` });
+    leftFooterElm.innerHTML = sanitizeTextByAvailableSanitizer(this.gridOptions, this.customFooterOptions.leftFooterText || '');
+    footerElm.appendChild(leftFooterElm);
     footerElm.appendChild(this.createFooterRightContainer());
     this._footerElement = footerElm;
 

diff --git a/packages/custom-tooltip-plugin/src/slickCustomTooltip.ts b/packages/custom-tooltip-plugin/src/slickCustomTooltip.ts
@@ -316,7 +316,8 @@ export class SlickCustomTooltip {
    * also clear the "title" attribute from the grid div text content so that it won't show also as a 2nd browser tooltip
    */
   protected renderRegularTooltip(formatterOrText: Formatter | string | undefined, cell: { row: number; cell: number; }, value: any, columnDef: Column, item: any) {
-    const tmpDiv = createDomElement('div', { innerHTML: this.parseFormatterAndSanitize(formatterOrText, cell, value, columnDef, item) });
+    const tmpDiv = document.createElement('div');
+    tmpDiv.innerHTML = this.parseFormatterAndSanitize(formatterOrText, cell, value, columnDef, item);
 
     let tooltipText = columnDef?.toolTip ?? '';
     let tmpTitleElm: HTMLDivElement | null | undefined;