Skip to content

Commit

Permalink
fix: sanitize query by default
Browse files Browse the repository at this point in the history
  • Loading branch information
virkt25 committed Aug 15, 2018
1 parent 99bca4b commit ee24cd0
Show file tree
Hide file tree
Showing 2 changed files with 113 additions and 10 deletions.
44 changes: 34 additions & 10 deletions lib/mongodb.js
Original file line number Diff line number Diff line change
Expand Up @@ -840,19 +840,22 @@ function idIncluded(fields, idName) {
return true;
}

MongoDB.prototype.buildWhere = function(model, where) {
MongoDB.prototype.buildWhere = function(model, where, options) {
var self = this;
var query = {};
if (where === null || typeof where !== 'object') {
return query;
}

where = sanitizeFilter(where, options);

var idName = self.idName(model);
Object.keys(where).forEach(function(k) {
var cond = where[k];
if (k === 'and' || k === 'or' || k === 'nor') {
if (Array.isArray(cond)) {
cond = cond.map(function(c) {
return self.buildWhere(model, c);
return self.buildWhere(model, c, options);
});
}
query['$' + k] = cond;
Expand Down Expand Up @@ -961,6 +964,7 @@ MongoDB.prototype.buildSort = function(model, order, options) {
}
}
if (order) {
order = sanitizeFilter(order, options);
var keys = order;
if (typeof keys === 'string') {
keys = keys.split(',');
Expand Down Expand Up @@ -1217,7 +1221,7 @@ MongoDB.prototype.all = function all(model, filter, options, callback) {
var idName = self.idName(model);
var query = {};
if (filter.where) {
query = self.buildWhere(model, filter.where);
query = self.buildWhere(model, filter.where, options);
}
var fields = filter.fields;

Expand Down Expand Up @@ -1308,7 +1312,8 @@ MongoDB.prototype.destroyAll = function destroyAll(
callback = where;
where = undefined;
}
where = self.buildWhere(model, where);
where = self.buildWhere(model, where, options);

this.execute(model, 'remove', where || {}, function(err, info) {
if (err) return callback && callback(err);

Expand All @@ -1335,7 +1340,7 @@ MongoDB.prototype.count = function count(model, where, options, callback) {
if (self.debug) {
debug('count', model, where);
}
where = self.buildWhere(model, where);
where = self.buildWhere(model, where, options);
this.execute(model, 'countDocuments', where, function(err, count) {
if (self.debug) {
debug('count.callback', model, err, count);
Expand Down Expand Up @@ -1506,9 +1511,9 @@ MongoDB.prototype.update = MongoDB.prototype.updateAll = function updateAll(
}
var idName = this.idName(model);

where = self.buildWhere(model, where);
delete data[idName];
where = self.buildWhere(model, where, options);

delete data[idName];
data = self.toDatabase(model, data);

// Check for other operators and sanitize the data obj
Expand Down Expand Up @@ -1798,6 +1803,23 @@ MongoDB.prototype.isObjectIDProperty = function(model, prop, value) {
}
};

function sanitizeFilter(filter, options) {
options = Object.assign({}, options);
if (options && options.disableSanitization) return filter;
if (!filter || typeof filter !== 'object') return filter;

for (const key in filter) {
if (key === '$where' || key === 'mapReduce') {
debug(`sanitizeFilter: deleting ${key}`);
delete filter[key];
}
}

return filter;
}

exports.sanitizeFilter = sanitizeFilter;

/**
* Find a matching model instances by the filter or create a new instance
*
Expand All @@ -1808,12 +1830,14 @@ MongoDB.prototype.isObjectIDProperty = function(model, prop, value) {
* @param {Object} filter The filter
* @param {Function} [callback] The callback function
*/
function optimizedFindOrCreate(model, filter, data, callback) {
function optimizedFindOrCreate(model, filter, data, options, callback) {
var self = this;
if (self.debug) {
debug('findOrCreate', model, filter, data);
}

if (!callback) callback = options;

var idValue = self.getIdValue(model, data);
var idName = self.idName(model);

Expand All @@ -1836,10 +1860,10 @@ function optimizedFindOrCreate(model, filter, data, callback) {
id = self.coerceId(model, id);
filter.where._id = id;
}
query = self.buildWhere(model, filter.where);
query = self.buildWhere(model, filter.where, options);
}

var sort = self.buildSort(model, filter.order);
var sort = self.buildSort(model, filter.order, options);

this.collection(model).findOneAndUpdate(
query,
Expand Down
79 changes: 79 additions & 0 deletions test/mongodb.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ var should = require('./init.js');
var testUtils = require('../lib/test-utils');
var async = require('async');
var sinon = require('sinon');
var sanitizeFilter = require('../lib/mongodb').sanitizeFilter;

var GeoPoint = require('loopback-datasource-juggler').GeoPoint;

Expand Down Expand Up @@ -717,6 +718,52 @@ describe('mongodb connector', function() {
});
});

it('should not return data for nested `$where` in where', function(done) {
Post.create({title: 'Post1', content: 'Post1 content'}, (err, p1) => {
Post.create({title: 'Post2', content: 'Post2 content'}, (err2, p2) => {
Post.create({title: 'Post3', content: 'Post3 data'}, (err3, p3) => {
Post.find({where: {$where: 'function() {return this.content.contains("content")}'}}, (err, p) => {
should.not.exist(err);
p.length.should.be.equal(3);
done();
});
});
});
});
});

it('should allow $where in where with options.disableSanitization', function(done) {
Post.create({title: 'Post1', content: 'Post1 content'}, (err, p1) => {
Post.create({title: 'Post2', content: 'Post2 content'}, (err2, p2) => {
Post.create({title: 'Post3', content: 'Post3 data'}, (err3, p3) => {
Post.find(
{where: {$where: 'function() {return this.content.contains("content")}'}},
{disableSanitization: true},
(err, p) => {
should.not.exist(err);
p.length.should.be.equal(2);
done();
}
);
});
});
});
});

it('does not execute a nested `$where`', function(done) {
Post.create({title: 'Post1', content: 'Post1 content'}, (err, p1) => {
Post.create({title: 'Post2', content: 'Post2 content'}, (err2, p2) => {
Post.create({title: 'Post3', content: 'Post3 data'}, (err3, p3) => {
Post.find({where: {content: {$where: 'function() {return this.content.contains("content")}'}}}, (err, p) => {
should.not.exist(err);
p.length.should.be.equal(0);
done();
});
});
});
});
});

it('should allow to find by id using where inq', function(done) {
Post.create({title: 'Post1', content: 'Post1 content'}, function(
err,
Expand Down Expand Up @@ -3218,6 +3265,38 @@ describe('mongodb connector', function() {
});
});

context('sanitizeFilter()', () => {
it('returns filter if not an object', () => {
const input = false;
const out = sanitizeFilter(input);
out.should.equal(input);
});

it('returns the filter if options.disableSanitization is true', () => {
const input = {key: 'value', $where: 'a value'};
const out = sanitizeFilter(input, {disableSanitization: true});
out.should.deepEqual(input);
});

it('removes `$where` property', () => {
const input = {key: 'value', $where: 'a value'};
const out = sanitizeFilter(input);
out.should.deepEqual({key: 'value'});
});

it('does not remove properties with `$` in it', () => {
const input = {key: 'value', $where: 'a value', random$key: 'random'};
const out = sanitizeFilter(input);
out.should.deepEqual({key: 'value', random$key: 'random'});
});

it('removes `mapReduce` property in the object', () => {
const input = {key: 'value', random$key: 'a value', mapReduce: 'map'};
const out = sanitizeFilter(input);
out.should.deepEqual({key: 'value', random$key: 'a value'});
});
});

after(function(done) {
User.destroyAll(function() {
Post.destroyAll(function() {
Expand Down

0 comments on commit ee24cd0

Please sign in to comment.