From 8296736f9a6fda7f87f8f4dd83eb93a5d583c101 Mon Sep 17 00:00:00 2001 From: gunce Date: Fri, 13 Oct 2023 23:40:25 +0300 Subject: [PATCH 1/5] fix: sanitize data for xss --- client/main.js | 35 ++++++++++++++++++++++++++++++++++- package.js | 1 + tests/sanitize.js | 12 ++++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 tests/sanitize.js diff --git a/client/main.js b/client/main.js index 0d59740..0150d6b 100755 --- a/client/main.js +++ b/client/main.js @@ -35,6 +35,38 @@ Tabular.getRecord = function (name, collection) { return Tabular.getTableRecordsCollection(collection._connection).findOne(name); }; +Tabular.sanitize = function (data) { + // Sanitizer function to escape HTML entities + const _escapeHTML = (unsafe) => { + return unsafe.replace(/[&<"']/g, (match) => { + switch (match) { + case '&': + return '&'; + case '<': + return '<'; + case '>': + return '>'; + case '"': + return '"'; + case "'": + return '''; + default: + return match; + } + }); + }; + + for (const element of data) { + for (const key in element) { + if (element.hasOwnProperty(key)) { + element[key] = _escapeHTML(element[key].toString()); + } + } + } + + return data; +}; + Template.tabular.helpers({ atts() { // We remove the "table" and "selector" attributes and assume the rest belong @@ -85,6 +117,7 @@ Template.tabular.onRendered(function () { // the first subscription, which will then trigger the // second subscription. + template.tabular.data = Tabular.sanitize(template.tabular.data); //console.log('data', template.tabular.data); // Update skip @@ -187,7 +220,7 @@ Template.tabular.onRendered(function () { var data = Template.currentData(); //console.log('currentData autorun', data); - + // if we don't have data OR the selector didn't actually change return out if (!data || (data.selector && template.tabular.selector === data.selector)) return; diff --git a/package.js b/package.js index dcecf31..337c147 100755 --- a/package.js +++ b/package.js @@ -63,6 +63,7 @@ Package.onTest(function(api) { api.addFiles('tests/reusedFunctions.js', 'client'); api.addFiles([ 'tests/util.js', + 'tests/sanitize.js', 'tests/mongoDBQuery.js', 'tests/utilIntegration.js' ], 'client' ); diff --git a/tests/sanitize.js b/tests/sanitize.js new file mode 100644 index 0000000..5771306 --- /dev/null +++ b/tests/sanitize.js @@ -0,0 +1,12 @@ +import Tabular from "../common/Tabular.js"; + +Tinytest.add('Sanitize - clear data of an array object', function (test) { + const data = [{ + name: ``, + surname: 'doe' + }] + + const sanitizedData = Tabular.sanitize(data); + + LogResults(data, "<script>alert("xss")</script>", sanitizedData[0].name, test) +}) From 99eda585c515e000daa26485f8f59fedb9dd320a Mon Sep 17 00:00:00 2001 From: gunce Date: Tue, 31 Oct 2023 10:47:41 +0300 Subject: [PATCH 2/5] add: xss doc --- README.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/README.md b/README.md index 8bc33a9..3179e86 100755 --- a/README.md +++ b/README.md @@ -377,6 +377,22 @@ TabularTables.Books = new Tabular.Table({ If you need to be sure that certain fields are never published or if different users can access different fields, use `allowFields`. Otherwise just use `allow`. +*XSS: Tabular sanitizes strings completely and objects partially. If you are trying to render an object as data, you should take required actions to prevent XSS.* +```js +{ + title: 'Customer', + data: 'customer', // an object + render(data){ + if (!data) { + return ''; + } + + const name = yourSanitizeFunction(data.name); + return `${name}}`; + } +} +``` + ## Caching the Documents By default, a normal `Meteor.subscribe` is used for the current page's table data. This subscription is stopped and a new one replaces it whenever you switch pages. This means that if your table shows 10 results per page, your client collection will have 10 documents in it on page 1. When you switch to page 2, your client collection will still have only 10 documents in it, but they will be the next 10. From 3328277d9e8c2587b00a2732f10c10a9747b5b5d Mon Sep 17 00:00:00 2001 From: gunce Date: Tue, 31 Oct 2023 10:52:14 +0300 Subject: [PATCH 3/5] add: sanitizer function --- client/main.js | 86 ++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 62 insertions(+), 24 deletions(-) diff --git a/client/main.js b/client/main.js index 0150d6b..a41e36a 100755 --- a/client/main.js +++ b/client/main.js @@ -36,8 +36,26 @@ Tabular.getRecord = function (name, collection) { }; Tabular.sanitize = function (data) { - // Sanitizer function to escape HTML entities - const _escapeHTML = (unsafe) => { + const _sanitizeJson = (unsafe) => { + if (typeof unsafe !== 'string') { + return unsafe; + } + + return unsafe.replace(/[&<"']/g, (match) => { + switch (match) { + case '&': + return '&'; + case '<': + return '<'; + case '>': + return '>'; + default: + return match; + } + }); + }; + + const _sanitizeString = (unsafe) => { return unsafe.replace(/[&<"']/g, (match) => { switch (match) { case '&': @@ -56,15 +74,35 @@ Tabular.sanitize = function (data) { }); }; - for (const element of data) { - for (const key in element) { - if (element.hasOwnProperty(key)) { - element[key] = _escapeHTML(element[key].toString()); - } + const _isJson = function (value) { + value = typeof value !== 'string' + ? JSON.stringify(value) + : value; + + try { + value = JSON.parse(value); + } catch (e) { + return false; } - } - return data; + return typeof value === 'object' && value !== null; + }; + + const sanitizeData = (unsafeArray) => { + unsafeArray.forEach(unsafeObject => { + for (const key in unsafeObject) { + if (_isJson(unsafeObject[key])) { + unsafeObject[key] = _sanitizeJson(unsafeObject[key]); + } else if (typeof unsafeObject[key] === 'string') { + unsafeObject[key] = _sanitizeString(unsafeObject[key]); + } + } + }); + + return unsafeArray; + }; + + return sanitizeData(data); }; Template.tabular.helpers({ @@ -179,17 +217,17 @@ Template.tabular.onRendered(function () { }).replaceWith(newText); } $('#' + tableId + '_filter input') - .unbind() - .bind('keyup change', function (event) { - if (!table) return; - if (event.keyCode === 13 || this.value === '') { - replaceSearchLabel(table.i18n('search')); - table.search(this.value).draw(); - } - else { - replaceSearchLabel(table.i18n('Press enter to filter')); - } - }); + .unbind() + .bind('keyup change', function (event) { + if (!table) return; + if (event.keyCode === 13 || this.value === '') { + replaceSearchLabel(table.i18n('search')); + table.search(this.value).draw(); + } + else { + replaceSearchLabel(table.i18n('Press enter to filter')); + } + }); } }, headerCallback(headerRow) { @@ -354,8 +392,8 @@ Template.tabular.onRendered(function () { // In some cases, there is no point in subscribing to nothing if (_.isEmpty(tableInfo) || - template.tabular.recordsTotal === 0 || - template.tabular.recordsFiltered === 0) { + template.tabular.recordsTotal === 0 || + template.tabular.recordsFiltered === 0) { return; } @@ -512,8 +550,8 @@ Template.tabular.onDestroyed(function () { Session.set('Tabular.LastSkip', 0); // Run a user-provided onUnload function if (this.tabular && - this.tabular.tableDef && - typeof this.tabular.tableDef.onUnload === 'function') { + this.tabular.tableDef && + typeof this.tabular.tableDef.onUnload === 'function') { this.tabular.tableDef.onUnload(); } From a67a67a54010c0f95411954f21ae4979fb8d99f7 Mon Sep 17 00:00:00 2001 From: gunce Date: Tue, 31 Oct 2023 10:58:44 +0300 Subject: [PATCH 4/5] feat: linting reverted --- client/main.js | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/client/main.js b/client/main.js index a41e36a..6ff142a 100755 --- a/client/main.js +++ b/client/main.js @@ -217,17 +217,17 @@ Template.tabular.onRendered(function () { }).replaceWith(newText); } $('#' + tableId + '_filter input') - .unbind() - .bind('keyup change', function (event) { - if (!table) return; - if (event.keyCode === 13 || this.value === '') { - replaceSearchLabel(table.i18n('search')); - table.search(this.value).draw(); - } - else { - replaceSearchLabel(table.i18n('Press enter to filter')); - } - }); + .unbind() + .bind('keyup change', function (event) { + if (!table) return; + if (event.keyCode === 13 || this.value === '') { + replaceSearchLabel(table.i18n('search')); + table.search(this.value).draw(); + } + else { + replaceSearchLabel(table.i18n('Press enter to filter')); + } + }); } }, headerCallback(headerRow) { @@ -258,7 +258,6 @@ Template.tabular.onRendered(function () { var data = Template.currentData(); //console.log('currentData autorun', data); - // if we don't have data OR the selector didn't actually change return out if (!data || (data.selector && template.tabular.selector === data.selector)) return; @@ -392,8 +391,8 @@ Template.tabular.onRendered(function () { // In some cases, there is no point in subscribing to nothing if (_.isEmpty(tableInfo) || - template.tabular.recordsTotal === 0 || - template.tabular.recordsFiltered === 0) { + template.tabular.recordsTotal === 0 || + template.tabular.recordsFiltered === 0) { return; } @@ -550,8 +549,8 @@ Template.tabular.onDestroyed(function () { Session.set('Tabular.LastSkip', 0); // Run a user-provided onUnload function if (this.tabular && - this.tabular.tableDef && - typeof this.tabular.tableDef.onUnload === 'function') { + this.tabular.tableDef && + typeof this.tabular.tableDef.onUnload === 'function') { this.tabular.tableDef.onUnload(); } From 5dcbf4b7468e4112d608080e20d94e0b34fce680 Mon Sep 17 00:00:00 2001 From: gunce Date: Tue, 28 Nov 2023 21:15:16 +0300 Subject: [PATCH 5/5] feat: Object.keys usage --- client/main.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/main.js b/client/main.js index 6ff142a..2dd55de 100755 --- a/client/main.js +++ b/client/main.js @@ -90,13 +90,13 @@ Tabular.sanitize = function (data) { const sanitizeData = (unsafeArray) => { unsafeArray.forEach(unsafeObject => { - for (const key in unsafeObject) { + Object.keys(unsafeObject).forEach(key => { if (_isJson(unsafeObject[key])) { unsafeObject[key] = _sanitizeJson(unsafeObject[key]); } else if (typeof unsafeObject[key] === 'string') { unsafeObject[key] = _sanitizeString(unsafeObject[key]); } - } + }); }); return unsafeArray;