fix: sanitize query by default

Author: virkt25

lib/mongodb.js

@@ -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;
@@ -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(',');
@@ -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;
 
@@ -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);
 
@@ -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);
@@ -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
@@ -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
  *
@@ -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);
 
@@ -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,

test/mongodb.test.js

@@ -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;
 
@@ -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,
@@ -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() {