Check-Checker Checks Your Checks

Written by Pete Corey on Jul 27, 2015.

I’ve been shouting about why you should check all of the user provided arguments in your Meteor applications for months now. Working with unchecked arguments can lead to a variety of serious security issues in your application.

But still, I find myself coming into client projects and security assessments where I see developers forgetting to check their arguments!

The audit-argument-checks is a great package designed to get people to check all of their method and publication arguments. Unfortunately, it has its shortcomings.

audit-argument-checks will only tell you that you’re missing check coverage for a method or publication at runtime, when that method or publication is called. The package politely informs you of this missing coverage by throwing an exception and killing the current method or publication.

What’s worse, audit-argument-checks is not a debugOnly package, so these exceptions will continue to be thrown in production releases, potentially breaking your application (arguably for good reason).

Wouldn’t it be great if we could get a report of missing checks at startup, rather than through exceptions at runtime? Now you can! Check out my newly released east5th:check-checker package.

meteor add east5th:check-checker

check-checker was born of my need to quickly get informed about the state of argument checking in an application during a security assessment. It’s built on top of ESLint, and uses static analysis techniques to find all method and publication declarations in a Meteor application. If check is not called on a method or publication argument within the body of the handler, a warning is shown in the server logs.

Imagine you have a file in your Meteor project, example.js, that contains method and publication declarations with unchecked arguments:

if (Meteor.isServer) {
    foo: function(bar) {
      return MyCollection.find();

    test: function(rab, oof) {
      SensitiveDocuments.update(rab, oof);

After adding check-checker, you’ll see the following warning in your server logs after the application starts:

   Method 'foo' has an unchecked argument: bar
   Publication 'baz' has an unchecked argument: rab
   Publication 'baz' has an unchecked argument: oof

The goal of check-checker is to make it easier for developers to quickly find where they’re lacking check coverage. The faster you can find the chinks in your armor, the faster you can secure your application.

Exploiting findOne to Aggregate Collection Data

Written by Pete Corey on Jul 21, 2015.

After watching Rob Conery’s awesome series of screencasts detailing how to build an eCommerce application using Meteor, I was excited to dig into his code. Along with finding a few other security vulnerabilities, I had fun playing with an interesting way of exploiting a findOne query to aggregate data from a Mongo collection.

To get started, take a look at this getCart method defined in shopping_cart.js:30-32:

getCart : function(userKey){
  return Carts.getCart(userKey);

And the corresponding Carts.getCart method in carts.js:

Carts.getCart = function(userKey){
  var cart = Carts.findOne({userKey : userKey});
  return cart;

The first thing you’ll notice (I hope) is that the userKey argument isn’t being checked. Your horror may be tempered, though, when you notice that it’s being passed into a findOne query instead of a find.

What’s the worst thing a hacker could do? Sure, they might get a single random Cart by passing in a query like {$gt: ''}, but it’s not like they can get at our whole collection, right?

… Right?!

Well, it turns out you can easily pull down all data from a collection using a findOne query. Take a look at some code a malicious user could run in their browser’s console to do just that:

var carts = [];
function getCartAndSave(userKeys) {'getCart', {$nin: userKeys}, function(e, r) {
        if (e || !r || !r._id) {

And just like that, the entire carts collection has been pulled down to the client. Let’s dig into the code to see what’s going on.

The key here is the $nin query operator. We begin by calling getCart and asking for a Cart who’s userKey is not in ($nin) the array ['']. This will return a random Cart. We push this cart’s userKey onto the array and ask for a Cart who’s userKey is not in that array. This will return another random Cart from the collection that we haven’t yet seen. We repeat this process until there are no more Carts to find, and we’ve aggregated all of the Carts on the client.

Now Mr. Malicious User can take his time perusing your potentially sensitive data.

Like most Meteor security issues, the fix for this is to check your user-provided arguments:

getCart : function(userKey){
  check(userKey, String);
  return Carts.getCart(userKey);

Now, a malicious user can’t pass an object into the getCart method. They may only pass a String, and will only be able to find a Cart if they know its userKey.

Never allow users to pass arbitrary data into your queries!

Why Is Rename Disallowed?

Written by Pete Corey on Jul 14, 2015.

Have you ever tried to use $rename in a collection update from the client? If so, you’ve probably noticed this error:

Access denied. Operator $rename not allowed in a restricted collection.

If we dig into Meteor’s source, we can see that $rename is (currently) the only disallowed modifier. Why is that? To put it bluntly, the Mongo $rename operator is a lot like your cousin’s kid at your last family reunion - easy to forget about, but without constant supervision it can wreak havoc.

It can be especially easy to forget about $rename operators when building collection validators that depend on validating modifier objects.

A Validator Example

Take a look at the following update validator. It’s intended to allow a user to $set the userId on a Posts object, but only if the value you’re assigning it matches their current user’s ID:

  update: function(userId, doc, fields, modifier) {
    for (var key in modifier) {
      if (modifier[key].hasOwnProperty("userId")) {
        if (key === "$set") {
          if (modifier[key].userId !== userId) {
            return false;
        } else {
          return false;
    return true;

In its current form, this is a solid validator. We’re checking if the user is running a modifier operation on userId. If they are, we check if it’s a $set. If it’s a $set, we verify that the value they’re setting userId to matches the current user’s ID. All other operations against userId are disallowed. Great!

Would this still be secure if $rename were an allowed operator? Consider if it were and we ran the following update:

Posts.update(post._id, {
  $rename: {
    title: 'userId'

Now our modifier isn’t directly operating on userId. It’s operating on title, which we’re assuming we have permission to modify. In this update, title will be renamed to userId, effectively dumping the value of title into the userId field.

This means that if $rename were an allowed operator and we were using this validator, users would have the power to set userId to any arbitrary value. Malicious users could inject posts into user users’ accounts. This would be a bad thing!

No Place On The Client

The $rename operator has valid uses. It can be great for updating schemas during migrations, and doing wholesale data transformations. However, these use cases are almost never carried out through the client layer. Instead, they’re done entirely on the backend.

As we’ve seen, it can be difficult to reason about the $rename operator when writing collection validators. By effectively reversing its field list when compared to all other Mongo operators, it can easily slip through the cracks in your validation and expose potentially dangerous vulnerabilities.

Rather than expose that potential risk to all Meteor applications in exchange for functionality that arguably shouldn’t exist on the client, it seems the Meteor team decided to disallow the $rename operation on client-side updates.

The Threat Persists

However, $rename operators can be used on the server. This means that vulnerabilities exposed through incomplete validation can still exist in the wild. Take a look at this method from a real-world project I was digging into recently:

editUserProfile: function (modifier) {
  var user = Meteor.user(),
      schema = Users.simpleSchema()._schema;
  _.each(modifier, function (operation) {
    _.keys(operation).forEach(function (fieldName) {
      var field = schema[fieldName];
      if (!Users.can.edit(user, field, user)) {
        throw new Meteor.Error('Not allowed!');

This method loops over each field of the provided modifier and checks if it is an editable field according to the Users schema. If it’s not editable and it’s trying to be modified, an exception is thrown.

For this example, let’s assume that there is an admin field in the Users schema that is un-editable by users, and a profile.location field that is editable but optional. Normally, a direct update to admin would result in an exception, but what happens if we run the following method calls:'editUserProfile', {$set: {'profile.location': 'truthy'}});'editUserProfile', {$rename: {'profile.location': 'admin'}});

Bam! We’re an admin!

We’re making two calls to editUserProfile. The first is setting the optional location field on our profile to "truthy". The next is renaming the profile.location to admin, which dumps our "truthy" value into admin. Since this system makes loose checks against the truthiness of the admin field, this was all we needed to escalate our privileges. This is a very bad thing!

Sealing The Cracks

It is rarely a good idea to pass a user-provided modifier object directly into a collection query running on the server. If your system is doing this, be sure that you’re whitelisting valid modifiers, rather than blacklisting bad operators.

A declarative solution to the last example’s flaw may be to extend the Users schema with an allowedOperators field. The Users.can.edit method would be passed the current operator along with the current field and determine if the schema allows the combination.

With this solution, the profile.location field would have only had $set in its allowedOperators. An attempt to $rename profile.location into admin would have failed. Success!