Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: sanitize data for xss #450

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<a href="/customer/${data._id}">${name}}</a>`;
}
}
```

## 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.
Expand Down
72 changes: 71 additions & 1 deletion client/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,76 @@ Tabular.getRecord = function (name, collection) {
return Tabular.getTableRecordsCollection(collection._connection).findOne(name);
};

Tabular.sanitize = function (data) {
const _sanitizeJson = (unsafe) => {
if (typeof unsafe !== 'string') {
return unsafe;
}

return unsafe.replace(/[&<"']/g, (match) => {
switch (match) {
case '&':
return '&amp;';
case '<':
return '&lt;';
case '>':
return '&gt;';
default:
return match;
}
});
};

const _sanitizeString = (unsafe) => {
return unsafe.replace(/[&<"']/g, (match) => {
switch (match) {
case '&':
return '&amp;';
case '<':
return '&lt;';
case '>':
return '&gt;';
case '"':
return '&quot;';
case "'":
return '&#039;';
default:
return match;
}
});
};

const _isJson = function (value) {
value = typeof value !== 'string'
? JSON.stringify(value)
: value;

try {
value = JSON.parse(value);
} catch (e) {
return false;
}

return typeof value === 'object' && value !== null;
};

const sanitizeData = (unsafeArray) => {
unsafeArray.forEach(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;
};

return sanitizeData(data);
};

Template.tabular.helpers({
atts() {
// We remove the "table" and "selector" attributes and assume the rest belong
Expand Down Expand Up @@ -85,6 +155,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
Expand Down Expand Up @@ -187,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;

Expand Down
1 change: 1 addition & 0 deletions package.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' );
Expand Down
12 changes: 12 additions & 0 deletions tests/sanitize.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import Tabular from "../common/Tabular.js";

Tinytest.add('Sanitize - clear data of an array object', function (test) {
const data = [{
name: `<script>alert("xss")</script>`,
surname: 'doe'
}]

const sanitizedData = Tabular.sanitize(data);

LogResults(data, "&lt;script>alert(&quot;xss&quot;)&lt;/script>", sanitizedData[0].name, test)
})