Skip to content

Commit

Permalink
feat: add basic html sanitizer w/regex to avoid xss scripting attack (#…
Browse files Browse the repository at this point in the history 
…652)
  • Loading branch information
@ghiscoding
ghiscoding authored Oct 22, 2021
1 parent c6cfe18 commit ffc682b
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 18 deletions.
15 changes: 5 additions & 10 deletions plugins/slick.customtooltip.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
* var customTooltipPlugin = new Slick.Plugins.CustomTooltip(columns, grid options);
*
* Available plugin options (same options are available in both column definition and/or grid options)
*
*
* Example 1 - via Column Definition
* var columns = [
* {
Expand All @@ -21,7 +21,7 @@
* }
* }
* ];
*
*
* OR Example 2 - via Grid Options (for all columns), NOTE: the column definition tooltip options will win over the options defined in the grid options
* var gridOptions = {
* enableCellNavigation: true,
Expand Down Expand Up @@ -466,9 +466,9 @@
if (typeof formatterOrText === 'function') {
var tooltipText = formatterOrText(cell.row, cell.cell, value, columnDef, item, _grid);
var formatterText = (typeof tooltipText === 'object' && tooltipText && tooltipText.text) ? tooltipText.text : (typeof tooltipText === 'string' ? tooltipText : '');
return sanitizeHtmlString(formatterText);
return _grid.sanitizeHtmlString(formatterText);
} else if (typeof formatterOrText === 'string') {
return sanitizeHtmlString(formatterOrText);
return _grid.sanitizeHtmlString(formatterOrText);
}
return '';
}
Expand All @@ -485,7 +485,7 @@
outputText = (_cellTooltipOptions.tooltipTextMaxLength && outputText.length > _cellTooltipOptions.tooltipTextMaxLength) ? outputText.substr(0, _cellTooltipOptions.tooltipTextMaxLength - 3) + '...' : outputText;

if (!tooltipText || (_cellTooltipOptions && _cellTooltipOptions.renderRegularTooltipAsHtml)) {
_tooltipElm.innerHTML = sanitizeHtmlString(outputText);
_tooltipElm.innerHTML = _grid.sanitizeHtmlString(outputText);
_tooltipElm.style.whiteSpace = (_cellTooltipOptions && _cellTooltipOptions.whiteSpace) || _defaultOptions.whiteSpace;
} else {
_tooltipElm.textContent = outputText || '';
Expand Down Expand Up @@ -531,11 +531,6 @@
_options = $.extend({}, _options, newOptions);
}

/** basic html sanitizer to avoid xss */
function sanitizeHtmlString(dirtyHtml) {
return typeof dirtyHtml === 'string' ? dirtyHtml.replace(/(\b)(on\S+)(\s*)=|javascript:([^>]*)[^>]*|(<\s*)(\/*)script([<>]*).*(<\s*)(\/*)script([<>]*)/gi, '') : '';
}

// Public API
$.extend(this, {
"init": init,
Expand Down
21 changes: 13 additions & 8 deletions slick.grid.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2424,13 +2424,13 @@ if (typeof Slick === "undefined") {
headerWidth = getColHeaderWidth(columnDef);
}
if (headerWidth === 0) {
headerWidth = (columnDef.width ? columnDef.width
: (columnDef.maxWidth ? columnDef.maxWidth
headerWidth = (columnDef.width ? columnDef.width
: (columnDef.maxWidth ? columnDef.maxWidth
: (columnDef.minWidth ? columnDef.minWidth : 20)
)
);
}

if (autoSize.colValueArray) {
// if an array of values are specified, just pass them in instead of data
maxColWidth = getColWidth(columnDef, $gridCanvas, autoSize.colValueArray);
Expand Down Expand Up @@ -3485,10 +3485,10 @@ if (typeof Slick === "undefined") {
function applyFormatResultToCellNode(formatterResult, cellNode, suppressRemove) {
if (formatterResult === null || formatterResult === undefined) { formatterResult = ''; }
if (Object.prototype.toString.call(formatterResult) !== '[object Object]') {
cellNode.innerHTML = formatterResult;
cellNode.innerHTML = sanitizeHtmlString(formatterResult);
return;
}
cellNode.innerHTML = formatterResult.text;
cellNode.innerHTML = sanitizeHtmlString(formatterResult.text);
if (formatterResult.removeClasses && !suppressRemove) {
$(cellNode).removeClass(formatterResult.removeClasses);
}
Expand Down Expand Up @@ -3995,7 +3995,7 @@ if (typeof Slick === "undefined") {
}

var x = document.createElement("div");
x.innerHTML = stringArray.join("");
x.innerHTML = sanitizeHtmlString(stringArray.join(""));

var processedRow;
var node;
Expand Down Expand Up @@ -4059,8 +4059,8 @@ if (typeof Slick === "undefined") {
var x = document.createElement("div"),
xRight = document.createElement("div");

x.innerHTML = stringArrayL.join("");
xRight.innerHTML = stringArrayR.join("");
x.innerHTML = sanitizeHtmlString(stringArrayL.join(""));
xRight.innerHTML = sanitizeHtmlString(stringArrayR.join(""));

for (var i = 0, ii = rows.length; i < ii; i++) {
if (( hasFrozenRows ) && ( rows[i] >= actualFrozenRow )) {
Expand Down Expand Up @@ -5924,6 +5924,10 @@ if (typeof Slick === "undefined") {
}
}

/** basic html sanitizer to avoid scripting attack */
function sanitizeHtmlString(dirtyHtml) {
return typeof dirtyHtml === 'string' ? dirtyHtml.replace(/(\b)(on\S+)(\s*)=|javascript:([^>]*)[^>]*|(<\s*)(\/*)script([<>]*).*(<\s*)(\/*)script(>*)|(&lt;)(\/*)(script|script defer)(.*)(&gt;|&gt;">)/gi, '') : dirtyHtml;
}

//////////////////////////////////////////////////////////////////////////////////////////////
// Debug
Expand Down Expand Up @@ -6116,6 +6120,7 @@ if (typeof Slick === "undefined") {
"getCellCssStyles": getCellCssStyles,
"getFrozenRowOffset": getFrozenRowOffset,
"setColumnHeaderVisibility": setColumnHeaderVisibility,
"sanitizeHtmlString": sanitizeHtmlString,

"init": finishInitialization,
"destroy": destroy,
Expand Down

3 comments on commit ffc682b

@syonfox
Copy link

@syonfox syonfox commented on ffc682b Dec 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this broke using onclick="glovalFunction()" get converted to "globalFunction()" causing prod to break :/ can this be an opt in? there must be away to allow this without xss

@ghiscoding
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@syonfox
another user said the same thing, but the thing is you're never supposed to put JS code in a formatter because that is an open door to XSS scripting attack, you're suppose to use the onClick event which is a more secure way of dealing with events.
Anyway, I did another PR #657 to optionally overwrite it, however we didn't release a new version yet.

@syonfox
Copy link

@syonfox syonfox commented on ffc682b Dec 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks I'll look into refactoring. Glad there will be an option to disable it in the future for edge cases. this snip-it may be especially usefully to those who find this.

#652 (comment)

Please sign in to comment.