CODEWITHSUNDEEP
javascriptmongodbmeteor
bbozo
bbozo

Reputation: 7301

Does Meteor have a mongo injection issue with the ACL?

Meteor docs http://docs.meteor.com/#dataandsecurity under "Input validation" say:

Meteor allows your methods and publish functions to take arguments of any JSON type. (In fact, Meteor's wire protocol supports EJSON, an extension of JSON which also supports other common types like dates and binary buffers.) JavaScript's dynamic typing means you don't need to declare precise types of every variable in your app, but it's usually helpful to ensure that the arguments that clients are passing to your methods and publish functions are of the type that you expect.

Meteor provides a lightweight library for checking that arguments and other values are the type you expect them to be. Simply start your functions with statements like check(username, String) or check(office, {building: String, room: Number}). The check call will throw an error if its argument is of an unexpected type.

Meteor also provides an easy way to make sure that all of your methods and publish functions validate all of their arguments. Just run meteor add audit-argument-checks and any method or publish function which skips checking any of its arguments will fail with an exception.

The mongo injection problem is explained in more details in the security talk: https://www.meteor.com/blog/2013/08/02/meteor-devshop-6-devshop-live-security-meteor-ui

So my questions are:

  1. one has to do check before calls to Mongo or risk sending untrusted JS objects into the db thus possibly opening up to a Mongo injection vulnerability? - YES, one should add audit-argument-checks in his app to protect against this
  2. this has to be done on all mongo actions and not just on find? - not sure
  3. if 1) or 2) are true, then how does this translate to the use of ACL-style database access rules (collection.allow/collection.deny) where calls to mongo api aren't filtered by manual check calls but rather left to the client? - you can add check to the allow/deny functions, but it depends on 2) if and when this is necessary

Upvotes: 4

Views: 851

Answers (2)

David Weldon
David Weldon

Reputation: 64312

  1. Yes. All kinds of bad things can happen if you blindly trust the values passed by the client to a method call.
  2. This gets into theory which probably very few people can give you a straight answer on, so why take the risk? I strongly recommend adding audit-argument-checks to all of your projects just to be safe.
  3. With respect to allow and deny, find calls are not at risk here because the data is already on the client (presumably your publish function should have already published the correct documents). I think the risk in this case has more to do with bad data than it does with security. It's easy enough to just add check calls inside the rules. For example:
Posts.allow({
  insert: function(userId, doc) {
    check(doc, {
      _id: String,
      message: String,
      createdAt: Date
    });
    return true;
  }
});

This will fail to insert a new post from the client if the document does not conform to the schema.

Upvotes: 5

sbking
sbking

Reputation: 7680

The allow and deny rules are checked on the server before it lets client queries run. You can absolutely use check inside your allow/deny functions, on both the client and the server. You don't write an allow rule for reads on the client, instead use check in your publish function.

Upvotes: 0

Related Questions