Merge pull request #157 from meteorrn/fix/collection-prototype-pollution

Fix: prototype pollution in collection

Author: jankapunkt

src/Collection.js

@@ -9,12 +9,12 @@ import { hasOwn, isPlainObject } from '../lib/utils.js';
  * @private
  * @type {object}
  */
-const observers = {};
+const observers = Object.create(null);
 /**
  * @private
  * @type {object}
  */
-const observersByComp = {};
+const observersByComp = Object.create(null);
 /**
  * Get the list of callbacks for changes on a collection
  * @param {string} type - Type of change happening.
@@ -45,7 +45,7 @@ export function getObservers(type, collection, newDocument) {
     });
   }
   // Find the observers related to the specific query
-  if (observersByComp[collection]) {
+  if (observersByComp[collection] && !(collection in {})) {
     let keys = Object.keys(observersByComp[collection]);
     for (let i = 0; i < keys.length; i++) {
       observersByComp[collection][keys[i]].callbacks.forEach(
@@ -184,6 +184,17 @@ export class Collection {
       localCollections.push(name);
     }
 
+    // XXX: apparently using a name that occurs in Object prototype causes
+    // Data.db[name] to return the full MemoryDb implementation from Minimongo
+    // instead of a collection.
+    // A respective issues has been opened: https://github.com/meteorrn/minimongo-cache
+    // Additionally, this is subject to prototype pollution.
+    if (name in {}) {
+      throw new Error(
+        `Object-prototype property ${name} is not a supported Collection name`
+      );
+    }
+
     if (!Data.db[name]) Data.db.addCollection(name);
 
     this._collection = Data.db[name];
@@ -232,14 +243,16 @@ export class Collection {
     // collection is changed it needs to be re-run
     if (Tracker.active && Tracker.currentComputation) {
       let id = Tracker.currentComputation._id;
-      observersByComp[this._name] = observersByComp[this._name] || {};
+      observersByComp[this._name] =
+        observersByComp[this._name] || Object.create(null);
       if (!observersByComp[this._name][id]) {
         let item = {
           computation: Tracker.currentComputation,
           callbacks: [],
         };
         observersByComp[this._name][id] = item;
       }
+
       let item = observersByComp[this._name][id];
 
       item.callbacks.push({

test/src/Collection.tests.js

@@ -1,25 +1,49 @@
 import { WebSocket } from 'mock-socket';
-import {
-  Collection,
-  localCollections,
-  runObservers,
-} from '../../src/Collection';
+import Mongo from '../../src/Mongo';
+import { endpoint, props } from '../testHelpers';
+import { localCollections, runObservers } from '../../src/Collection';
 import { expect } from 'chai';
 import Data from '../../src/Data';
 import DDP from '../../lib/ddp';
 import Random from '../../lib/Random';
 import { server } from '../hooks/mockServer';
+import Tracker from '../../src/Tracker';
+
+const Collection = Mongo.Collection;
+const objectProps = props({});
+const defaultProps = [
+  '_collection',
+  '_name',
+  '_transform',
+  'constructor',
+  'find',
+  'findOne',
+  'insert',
+  'update',
+  'remove',
+  'helpers',
+  '__defineGetter__',
+  '__defineSetter__',
+  'hasOwnProperty',
+  '__lookupGetter__',
+  '__lookupSetter__',
+  'isPrototypeOf',
+  'propertyIsEnumerable',
+  'toString',
+  'valueOf',
+  '__proto__',
+  'toLocaleString',
+];
 
 describe('Collection', function () {
-  const endpoint = 'ws://localhost:3000/websocket';
-
   // for proper collection tests we need the server to be active
 
   before(function () {
     if (!Data.ddp) {
       Data.ddp = new DDP({
         SocketConstructor: WebSocket,
         endpoint,
+        autoConnect: false,
       });
       Data.ddp.socket.on('open', () => {
         Data.ddp.socket.emit('message:in', { msg: 'connected' });
@@ -46,12 +70,17 @@ describe('Collection', function () {
   });
 
   describe('constructor', function () {
+    it('is exported via Mongo', () => {
+      expect(Mongo.Collection).to.equal(Collection);
+    });
     it('creates a new collection and one in Minimongo', function () {
       const name = Random.id(6);
       const c = new Collection(name);
       expect(c._name).to.equal(name);
       expect(c._transform).to.equal(null);
-      expect(Data.db.collections[name]).to.equal(c._collection);
+      expect(c._collection).to.equal(Data.db.collections[name]);
+      expect(c._collection.name).to.equal(name);
+      expect(c._collection.constructor.name).to.equal('Collection');
     });
     it('creates a local collection and a random counterpart in minimongo', function () {
       const c = new Collection(null);
@@ -85,6 +114,14 @@ describe('Collection', function () {
       c = new Collection(Random.id(), { transform });
       expect(c._transform({ _id })).to.deep.equal({ _id, foo: 'bar' });
     });
+    it('does not imply prototype pollution', () => {
+      objectProps.forEach((name) => {
+        expect(() => new Mongo.Collection(name)).to.throw(
+          `Object-prototype property ${name} is not a supported Collection name`
+        );
+        expect(props({})).to.deep.equal(objectProps);
+      });
+    });
   });
 
   describe('insert', function () {
@@ -173,6 +210,55 @@ describe('Collection', function () {
         done();
       });
     });
+    it('triggers a reactive observer', (done) => {
+      const collectionName = Random.id(6);
+      const c = new Mongo.Collection(collectionName);
+
+      Tracker.autorun((comp) => {
+        const doc = c.findOne({ foo: 1 });
+        if (doc) {
+          comp.stop();
+          done();
+        }
+      });
+
+      setTimeout(() => c.insert({ foo: 1 }), 50);
+    });
+    it('does not imply prototype pollution', (done) => {
+      const collectionName = Random.id(6);
+      const c = new Mongo.Collection(collectionName);
+      objectProps.forEach((name) => c.find(name).fetch());
+      expect(props({})).to.deep.equal(objectProps);
+      const insertDoc = {};
+      objectProps.forEach((prop) => {
+        insertDoc[prop] = 'foo';
+      });
+
+      Tracker.autorun((comp) => {
+        if (c.find().count() < 1) {
+          return;
+        }
+
+        c._name = '__proto__';
+        try {
+          c.find(insertDoc);
+        } catch {}
+        try {
+          expect(props({})).to.deep.equal(objectProps);
+        } catch (e) {
+          comp.stop();
+          return done(e);
+        }
+
+        comp.stop();
+        done();
+      });
+
+      setTimeout(() => {
+        c._name = collectionName;
+        c.insert(insertDoc);
+      }, 50);
+    });
   });
 
   describe('update', function () {

test/testHelpers.js

@@ -1,6 +1,8 @@
 import sinon from 'sinon';
 import Meteor from '../src/Meteor';
 
+export const endpoint = 'ws://localhost:3000/websocket';
+
 export const stubs = new Map();
 
 export const stub = (target, name, handler) => {
@@ -47,3 +49,13 @@ export const awaitDisconnected = async () => {
     }, 100);
   });
 };
+
+export const props = (obj) => {
+  let p = [];
+  for (; obj != null; obj = Object.getPrototypeOf(obj)) {
+    let op = Object.getOwnPropertyNames(obj);
+    for (let i = 0; i < op.length; i++)
+      if (p.indexOf(op[i]) === -1) p.push(op[i]);
+  }
+  return p;
+};