Skip to content

Commit

Permalink
fix(sanitizer): add optional grid option sanitizer anywhere possible (#9
Browse files Browse the repository at this point in the history
)
  • Loading branch information
ghiscoding authored Jul 10, 2020
1 parent c0b8ad9 commit a6c7997
Show file tree
Hide file tree
Showing 13 changed files with 132 additions and 37 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ npm run test:watch
- [x] Local Pagination
- [x] Grid Presets
- [ ] Preset Row Selections
- [ ] Preset Filters not working with Tree Data View
- [ ] Dynamically Add Columns
- [ ] Translations Support
- [ ] Tree Data
Expand Down
11 changes: 2 additions & 9 deletions packages/common/src/editors/selectEditor.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import * as DOMPurify from 'dompurify';

import { Constants } from '../constants';
import { FieldType } from './../enums/index';
import {
Expand All @@ -18,7 +16,7 @@ import {
SlickGrid,
} from './../interfaces/index';
import { CollectionService, findOrDefault, TranslaterService } from '../services/index';
import { charArraysEqual, getDescendantProperty, getTranslationPrefix, htmlEncode, setDeepValue } from '../services/utilities';
import { charArraysEqual, getDescendantProperty, getTranslationPrefix, htmlEncode, sanitizeTextByAvailableSanitizer, setDeepValue } from '../services/utilities';

/**
* Slickgrid editor class for multiple/single select lists
Expand Down Expand Up @@ -569,12 +567,7 @@ export class SelectEditor implements Editor {
if (isRenderHtmlEnabled) {
// sanitize any unauthorized html tags like script and others
// for the remaining allowed tags we'll permit all attributes
let sanitizedText = '';
if (this.gridOptions && typeof this.gridOptions.sanitizer === 'function') {
sanitizedText = this.gridOptions.sanitizer(optionText);
} else {
sanitizedText = (DOMPurify.sanitize(optionText, sanitizedOptions) || '').toString();
}
const sanitizedText = sanitizeTextByAvailableSanitizer(this.gridOptions, optionText, sanitizedOptions);
optionText = htmlEncode(sanitizedText);
}

Expand Down
6 changes: 2 additions & 4 deletions packages/common/src/filters/selectFilter.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import * as DOMPurify from 'dompurify';

import { Constants } from '../constants';
import { OperatorString, OperatorType, SearchTerm, } from '../enums/index';
import {
Expand All @@ -17,7 +15,7 @@ import {
SlickGrid
} from './../interfaces/index';
import { CollectionService } from '../services/collection.service';
import { getDescendantProperty, getTranslationPrefix, htmlEncode } from '../services/utilities';
import { getDescendantProperty, getTranslationPrefix, htmlEncode, sanitizeTextByAvailableSanitizer } from '../services/utilities';
import { TranslaterService } from '../services';

export class SelectFilter implements Filter {
Expand Down Expand Up @@ -317,7 +315,7 @@ export class SelectFilter implements Filter {
if (isRenderHtmlEnabled) {
// sanitize any unauthorized html tags like script and others
// for the remaining allowed tags we'll permit all attributes
const sanitizedText = (DOMPurify.sanitize(optionText, sanitizedOptions) || '').toString();
const sanitizedText = sanitizeTextByAvailableSanitizer(this.gridOptions, optionText, sanitizedOptions);
optionText = htmlEncode(sanitizedText);
}

Expand Down
12 changes: 6 additions & 6 deletions packages/common/src/formatters/hyperlinkFormatter.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import * as DOMPurify from 'dompurify';

import { Column, Formatter } from './../interfaces/index';
import { Column, Formatter, SlickGrid } from './../interfaces/index';
import { sanitizeTextByAvailableSanitizer } from '../services/utilities';

/**
* Takes an hyperlink cell value and transforms it into a real hyperlink, given that the value starts with 1 of these (http|ftp|https).
Expand All @@ -12,14 +11,15 @@ import { Column, Formatter } from './../interfaces/index';
* You can also optionally provide the hyperlink URL by using the generic params "hyperlinkUrl" in the column definition
* For example: { id: 'link', field: 'link', params: { hyperlinkText: 'Company Website', hyperlinkUrl: 'http://www.somewhere.com' } } will display "<a href="http://www.somewhere.com">Company Website</a>"
*/
export const hyperlinkFormatter: Formatter = (row: number, cell: number, value: any, columnDef: Column, dataContext: any) => {
export const hyperlinkFormatter: Formatter = (row: number, cell: number, value: any, columnDef: Column, dataContext: any, grid: SlickGrid) => {
const columnParams = columnDef && columnDef.params || {};
const gridOptions = (grid && typeof grid.getOptions === 'function') ? grid.getOptions() : {};

let displayedText = columnParams.hyperlinkText ? columnParams.hyperlinkText : value;
displayedText = DOMPurify.sanitize(displayedText || '');
displayedText = sanitizeTextByAvailableSanitizer(gridOptions, displayedText);

let outputLink = columnParams.hyperlinkUrl ? columnParams.hyperlinkUrl : value;
outputLink = DOMPurify.sanitize(outputLink || '');
outputLink = sanitizeTextByAvailableSanitizer(gridOptions, outputLink);

const matchUrl = outputLink.match(/^(http|ftp|https):\/\/[\w\-_]+(\.[\w\-_]+)+([\w\-\.,@?^=%&amp;:\/~\+#]*[\w\-\@?^=%&amp;\/~\+#])?/i);

Expand Down
67 changes: 67 additions & 0 deletions packages/common/src/services/__tests__/utilities.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
parseBoolean,
parseUtcDate,
sanitizeHtmlToText,
sanitizeTextByAvailableSanitizer,
setDeepValue,
thousandSeparatorFormatted,
titleCase,
Expand Down Expand Up @@ -1230,6 +1231,72 @@ describe('Service/Utilies', () => {
});
});

describe('sanitizeTextByAvailableSanitizer method', () => {
describe('use default DOMPurify sanitizer when no sanitizer exist', () => {
const gridOptions = {} as GridOption;

it('should return original value when input does not include any HTML tags', () => {
const input = 'foo bar';
const output = sanitizeTextByAvailableSanitizer(gridOptions, input);
expect(output).toBe('foo bar');
});

it('should return original value when input does not include any bad HTML tags', () => {
const input = '<div class="color: blue">Something</div>';
const output = sanitizeTextByAvailableSanitizer(gridOptions, input);
expect(output).toBe('<div class="color: blue">Something</div>');
});

it('should return empty string when some javascript script tags are included', () => {
const input = '<script>alert("Hello World")</script>';
const output = sanitizeTextByAvailableSanitizer(gridOptions, input);
expect(output).toBe('');
});

it('should return an empty <a> link tag when "javascript:" is part of the dirty html', () => {
const input = '<a href="javascript:alert(\"Hello World\")"></a>';
const output = sanitizeTextByAvailableSanitizer(gridOptions, input);
expect(output).toBe('<a></a>');
});
});

describe('use custom sanitizer when provided in the grid options', () => {
const gridOptions = {
sanitizer: (dirtyHtml) => (dirtyHtml.replace(/(\b)(on\S+)(\s*)=|javascript:([^>]*)[^>]*|(<\s*)(\/*)script([<>]*).*(<\s*)(\/*)script([<>]*)/gi, '')),
} as GridOption;

it('should return original value when input does not include any HTML tags', () => {
const input = 'foo bar';
const output = sanitizeTextByAvailableSanitizer(gridOptions, input);
expect(output).toBe('foo bar');
});

it('should return original value when input does not include any bad HTML tags', () => {
const input = '<div class="color: blue">Something</div>';
const output = sanitizeTextByAvailableSanitizer(gridOptions, input);
expect(output).toBe('<div class="color: blue">Something</div>');
});

it('should return empty string when some javascript script tags are included', () => {
const input = '<script>alert("Hello World")</script>';
const output = sanitizeTextByAvailableSanitizer(gridOptions, input);
expect(output).toBe('');
});

it('should return text without the word "javascript:" when that is part of the dirty html', () => {
const input = 'javascript:alert("Hello World")';
const output = sanitizeTextByAvailableSanitizer(gridOptions, input);
expect(output).toBe('');
});

it('should return an empty <a> link tag when "javascript:" is part of the dirty html', () => {
const input = '<a href="javascript:alert(\"Hello World\")"></a>';
const output = sanitizeTextByAvailableSanitizer(gridOptions, input);
expect(output).toBe('<a href="></a>');
});
});
});

describe('setDeepValue method', () => {
let obj = {};
beforeEach(() => {
Expand Down
20 changes: 20 additions & 0 deletions packages/common/src/services/utilities.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import * as DOMPurify_ from 'dompurify';
import * as moment_ from 'moment-mini';
const DOMPurify = DOMPurify_; // patch to fix rollup to work
const moment = moment_; // patch to fix rollup "moment has no default export" issue, document here https://github.com/rollup/rollup/issues/670

import { FieldType, OperatorString, OperatorType } from '../enums/index';
Expand Down Expand Up @@ -743,6 +745,24 @@ export function sanitizeHtmlToText(htmlString: string): string {
return temp.textContent || temp.innerText || '';
}

/**
* Sanitize possible dirty html string (remove any potential XSS code like scripts and others), we will use 2 possible sanitizer
* 1. optional sanitizer method defined in the grid options
* 2. DOMPurify sanitizer (defaults)
* @param gridOptions: grid options
* @param dirtyHtml: dirty html string
* @param domPurifyOptions: optional DOMPurify options when using that sanitizer
*/
export function sanitizeTextByAvailableSanitizer(gridOptions: GridOption, dirtyHtml: string, domPurifyOptions?: DOMPurify.Config & { RETURN_TRUSTED_TYPE: true; }): string {
let sanitizedText = '';
if (gridOptions && typeof gridOptions.sanitizer === 'function') {
sanitizedText = gridOptions.sanitizer(dirtyHtml || '');
} else {
sanitizedText = (DOMPurify.sanitize(dirtyHtml || '', domPurifyOptions || {}) || '').toString();
}
return sanitizedText;
}

/** Set the object value of deeper node from a given dot (.) notation path (e.g.: "user.firstName") */
export function setDeepValue<T = any>(obj: T, path: string | string[], value: any) {
if (typeof path === 'string') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ $row-selected-color: #d4f6d7; /*rgba(0, 149, 48, 0.2);*/

@import './slickgrid-theme-material.scss';

$link-color: #0099ff;
$link-color: #006DCC;

.cell-effort-driven {
text-align: center;
Expand Down
Binary file not shown.
8 changes: 4 additions & 4 deletions packages/vanilla-bundle/src/components/slick-footer.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import * as DOMPurify from 'dompurify';
import * as moment_ from 'moment-mini';
const moment = moment_; // patch to fix rollup "moment has no default export" issue, document here https://github.com/rollup/rollup/issues/670

Expand All @@ -10,6 +9,7 @@ import {
Metrics,
SlickGrid,
TranslaterService,
sanitizeTextByAvailableSanitizer,
} from '@slickgrid-universal/common';
import { BindingHelper } from '../services/binding.helper';

Expand Down Expand Up @@ -87,7 +87,7 @@ export class SlickFooterComponent {

const leftFooterElm = document.createElement('div');
leftFooterElm.className = `left-footer ${this.customFooterOptions.leftContainerClass}`;
leftFooterElm.innerHTML = DOMPurify.sanitize(this.customFooterOptions.leftFooterText || '', this._domPurifyOptions).toString();
leftFooterElm.innerHTML = sanitizeTextByAvailableSanitizer(this.gridOptions, this.customFooterOptions.leftFooterText || '');

const metricsElm = document.createElement('div');
metricsElm.className = 'metrics';
Expand All @@ -103,8 +103,8 @@ export class SlickFooterComponent {
footerElm.appendChild(metricsElm);
this._footerElement = footerElm;

if (gridParentContainerElm?.append && this._footerElement) {
gridParentContainerElm.append(this._footerElement);
if (gridParentContainerElm?.appendChild && this._footerElement) {
gridParentContainerElm.appendChild(this._footerElement);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export const SalesforceGlobalGridOptions: GridOption = {
headerMenu: {
hideFreezeColumnsCommand: false,
},
sanitizer: (dirtyHtml) => (dirtyHtml.replace(/(\b)(on\S+)(\s*)=|javascript|(<\s*)(\/*)script/gi, '')),
sanitizer: (dirtyHtml) => (dirtyHtml.replace(/(\b)(on\S+)(\s*)=|javascript:([^>]*)[^>]*|(<\s*)(\/*)script([<>]*).*(<\s*)(\/*)script([<>]*)/gi, '')),
headerRowHeight: 35,
rowHeight: 33,
eventNamingStyle: EventNamingStyle.lowerCaseWithoutOnPrefix,
Expand Down
9 changes: 6 additions & 3 deletions packages/vanilla-bundle/src/services/binding.helper.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { BindingService } from './binding.service';
import { BindingService, ElementBindingWithListener } from './binding.service';

export class BindingHelper {
private _observers: BindingService[] = [];
Expand All @@ -15,8 +15,11 @@ export class BindingHelper {

dispose() {
for (const observer of this._observers) {
for (const binding of observer.elementBindings) {
observer.unbind(binding.element, binding.event, binding.listener);
const elementBindings = observer.elementBindings as Array<ElementBindingWithListener>;
for (const binding of elementBindings) {
if (binding?.event && binding?.listener) {
observer.unbind(binding.element, binding.event, binding.listener);
}
}
}
}
Expand Down
21 changes: 13 additions & 8 deletions packages/vanilla-bundle/src/services/binding.service.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
import * as DOMPurify from 'dompurify';
import * as DOMPurify_ from 'dompurify';
const DOMPurify = DOMPurify_; // patch to fix rollup to work

interface Binding {
variable: any;
property: string;
}

interface ElementBinding {
export interface ElementBinding {
element: Element | null;
attribute: string;
}
export interface ElementBindingWithListener extends ElementBinding {
event: string;
listener: (val: any) => any;
}


/**
* Create 2 way Bindings for any variable that are primitive or object types, when it's an object type it will watch for property changes
* The following 2 articles helped in building this service:
Expand All @@ -23,7 +25,7 @@ export class BindingService {
_value: any = null;
_binding: Binding;
_property: string;
elementBindings: ElementBinding[] = [];
elementBindings: Array<ElementBinding | ElementBindingWithListener> = [];

constructor(binding: Binding) {
this._binding = binding;
Expand Down Expand Up @@ -67,11 +69,11 @@ export class BindingService {
* 2.1- we could also provide an extra callback method to execute when the event gets triggered
*/
bind(element: Element | null, attribute: string, eventName?: string, callback?: (val: any) => any) {
const binding: ElementBinding = { element, attribute, event: '', listener: () => { } };
const binding: ElementBinding | ElementBindingWithListener = { element, attribute };

if (element) {
if (eventName) {
element.addEventListener(eventName, () => {
const listener = () => {
const elmValue = element[attribute];
this.valueSetter(elmValue);
if (this._binding.variable.hasOwnProperty(this._binding.property) || this._binding.property in this._binding.variable) {
Expand All @@ -81,8 +83,11 @@ export class BindingService {
if (typeof callback === 'function') {
return callback(this.valueGetter());
}
});
binding.event = eventName;
};

(binding as ElementBindingWithListener).event = eventName;
(binding as ElementBindingWithListener).listener = listener;
element.addEventListener(eventName, listener);
}
this.elementBindings.push(binding);
element[attribute] = typeof this._value === 'string' ? DOMPurify.sanitize(this._value, {}) : this._value;
Expand Down
10 changes: 9 additions & 1 deletion packages/web-demo-vanilla-bundle/src/examples/example05.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,20 @@ export class Example5 {
},
registerExternalServices: [new ExcelExportService()],
enableFiltering: true,
showCustomFooter: true, // display some metrics in the bottom custom footer
customFooterOptions: {
// optionally display some text on the left footer container
leftFooterText: 'Grid created with <a href="https://github.com/ghiscoding/slickgrid-universal" target="_blank">Slickgrid-Universal</a>',
},
enableTreeData: true, // you must enable this flag for the filtering & sorting to work as expected
treeDataOptions: {
columnId: 'title',
levelPropName: 'indent',
parentPropName: 'parentId'
}
},
// presets: {
// filters: [{ columnId: 'percentComplete', searchTerms: ['50'], operator: '>=' }]
// }
};
}

Expand Down

0 comments on commit a6c7997

Please sign in to comment.