Mongo's Multi Parameter Saves the Day

Written by Pete Corey on May 18, 2015.

In the past, I’ve seen the multi parameter on MongoDB updates as an annoying inconvenience. Without fail, I’ll forget to add the flag when it’s needed, and waste a frustrating amount of time trying to deduce why my update isn’t behaving as expected. But, a recent trek through Telescope’s codebase revealed to me just how valuable it can be to default to updating a single item at a time. Come with me on a journey…

On a side note, Telescope is one of the highest quality open source Meteor projects I've seen to date. I highly recommend it as a platform!

Digging Into Telescope

The code that opened my proverbial eyes is the changeEmail method found in /server/users.js in Telescope. Take a look:

changeEmail: function (userId, newEmail) {
  var user = Meteor.users.findOne(userId);
  if (can.edit(Meteor.user(), user) !== true) {
    throw new Meteor.Error("Permission denied");
  }
  Meteor.users.update(
    userId,
    {$set: {
        emails: [{address: newEmail, verified: false}],
        email_hash: Gravatar.hash(newEmail),
        "profile.email": newEmail
      }
    }
  );
}

If you’ve read my previous posts, I hope you’ll immediately notice that userId and newEmail are not being checked. Can that be exploited? What happens if a malicious user, Mallory, decides to pass in a carefully crafted object as the userId?

Meteor.call(‘changeEmail', {_id: {$gte: Meteor.userId()}}, ‘mallory@is.evil');

This userId object, when used in a Mongo query, will return all users with IDs ordinally greater than or equal to the current user’s ID. We can roughly assume that this is about half of the users in the system. The list of returned users will always return the current user first, due to the index on the _id field.

In our changeEmail method, Meteor.users.findOne will happily accept our userId object and treat it as a query object. It will grab the first result of this query, which happens to be the current user (Mallory).

Next, the method checks if the current user has permission to edit the user found. Because the first user found is the current user, permission is granted. Proceed with the update!

You Get a New Email Address! And You Get a New Email Address!

If Mongo didn’t require explicitly setting the multi parameter, calling Meteor.users.update with our malicious userId object would result in a huge chunk of users having their emails changed to Mallory’s email address. This would be a Very Bad Thing™. He could easily reset their passwords through normal channels and take over their accounts!

But Not Really…

Thankfully, Mongo defaults to updating only a single object unless the multi flag is explicitly set to true. In this example, only the first result of the query, the current user, will be updated with the new email address. This means that the method functions as intended, and there is no vulnerability! Three cheers for the multi flag!

Final Thoughts

While I’m sure I’ll still continue to forget to add the multi flag when writing updates, I now have a newfound respect for this aspect of Mongo. This conservative default, which was initially built in with performance in mind, also acts as a protective mechanism to prevent us from inadvertently modifying more data than we intended.

Lastly, remember to always check your method and publication arguments! While it ended up not being a problem in this example, we nearly had a serious vulnerability on our hands. It’s always better to be safe than sorry.

Private Package Problems

Written by Pete Corey on May 11, 2015.

Recently I’ve been experiencing some pain points when trying to share private package between my Meteor projects.

Suppose you have two or more Meteor projects that share similar styles, components and functionality. Wouldn’t it be nice to bundle those shared bits into packages and use them across all of your projects?

The Problem With Packages

Packages are the solution! Well, partially…

Many articles have been written on the ease-of-use and power of Meteor’s packaging system. Unfortunately, when we’re writing closed source software, we’re not in the position to publish our private packages Meteor’s public package ecosystem. Our need for privacy means that we need to share our packages in a different way.

Git to the Rescue

My first attempt to solve this problem was to keep my private packages within private Git repositories. When an application needed one of these shared packages, I would clone the package into the project’s packages folder, and then add the package using the familiar meteor add command. Updates to the package could be made by manually running a git pull from within the package’s directory (goodbye meteor update).

cd packages
git clone https://github.com/pcorey/private-package.git
meteor add private-package
cd private-package
git pull

Finally, track the new package in the base project and commit the changes:

git add .
git commit -m "Added private package to project!"

This will work until a second developer checks out a fresh copy of the project. They’ll quickly notice that the packages/private-package is an empty directory! Where did our package go?

Unfortunately, Git noticed that the the packages/private-package directory was actually another Git repository, so it added the directory as an unmapped submodule, not a normal folder. This means that the base project isn’t concerned with tracking the contents of the packages/private-package directory, assuming that the submodule will handle that itself.

Git Submodules

Git submodules are the “accepted” standard for dealing with this kind of repository nesting. Git expects you to add and map these submodules through a special set of commands. Using our current example, to add private-package to our Meteor project we would:

git submodule add https://github.com/pcorey/private-package.git packages/private-package
meteor add private-package

This will pull the package down from its remote repository and set up the submodule mapping within Git. At this point, we can once again commit the changes to our base project. Another developer checking out a fresh copy of the project will now look at packages/private-package and see… an empty directory? Still?

One of the unfortunate subtleties of Git submodules is that they need to be initialized and updated on fresh checkouts:

git submodule init
git submodule update

Only then will the remote contents of each submodule be pulled into the project.

This means that the developer doing the checkout must be aware that submodules are being used by the project. This isn’t always the case, and often adds an unfortunate complexity when trying to introduce someone to a codebase.

Unfortunately, submodules have their fair share of quirks and problems.

“Fake” Submodules

My preferred method of sharing private Meteor packages between projects is a combination of the above two techniques, sometimes referred to as “fake” git submodules.

I begin by cloning my package into the packages folder, as before:

cd packages
git clone https://github.com/pcorey/private-package.git
meteor add private-package

There is a subtle step in how the package is added to my base project:

git add private-package/

The key here is the “/” at the end of the private-package path. Run a git status and you’ll notice that all of the private package’s files are now being tracked by the project. Additionally, if we cd into private-package and run a git status, we’ll see that the private-package is still operating under it’s own independent Git environment. The slash causes Git to treat this sub-repository as a normal directory and happily tracks all files within it without our base project!

Commit the changes to the base project and push them. Now, a new developer checking out a clean copy of the project will see that the private-package directory contains the contents of the package, as expected.

The only downside of this technique is that private-package in the fresh checkout no longer maintains a remote link to its private package repository. If changes need to be made in this package and pushed to the private package’s repository, the remote would have to be re-established:

git init
git remote add https://github.com/pcorey/private-package.git
git checkout master --force

Final Thoughts

My final thoughts are that this is a mess. I hope that the Meteor team adds some mechanism for adding and updating packages from private sources, rather than exclusively from its public package ecosystem. Maybe one day it will be as easy as meteor add and meteor update, but for now, these are the tools we have to work with.

Meteor Security in the Wild

Written by Pete Corey on May 5, 2015.

I was recently poking through the Meteor publications being used in a client project and I found an interesting vulnerability. Imagine an admin panel that shows a list of all users in the system. That page/route needs to subscribe to a publication that publishes all of the users, but only if the current user is an admin. We don’t want non-administrators having access to all of the user data in the system! Are you imagining? Good! Here’s the publication, as seen in the wild:

Meteor.publish('users', function(userId){
    if(Roles.userIsInRole(userId, 'admin')){
        return Meteor.users.find({}, {fields: {...});
    }
});

This publication takes an argument that is intended to be the current user’s ID. It would be subscribed to on the client like this:

Meteor.subscribe('users', Meteor.userId());

If you’re an astute observer, you may notice a few potential problems here. Let’s dig into them!

Guess the Admin ID

Since the userId is a user provided argument, and we’re not actually validating that the currently logged in user is the user associated with the provided ID, a malicious user could potentially just guess an administrator’s ID. Or, instead of guessing the ID, they may find it in other public data (posts, comments, profiles, etc…). They could easily subscribe to the publication right from their browser console:

Meteor.subscribe('users', '[spoofed admin ID]');

But that’s assuming that they know the publication exists, right? If the malicious user can never get to the admin route, they’ll never see the subscribe happen, and they’ll have no way of knowing that they can subscribe to it. Right? Wrong!

A quick search through the minified and concatenated JavaScript served to each client will show each subscription being made (search for ".subscribe("), even if it is happening behind some kind of protection mechanism. If any client can get to it, all clients can get to it.

BYO User Object

Take a look at lines 307 and 330 of this file in the alanning:meteor-roles package. You’ll notice that isUserInRole accepts either a user ID as a string, or the entire user object. Looking deeper, we can see that if a user object is passed in, it will return true if the passed in role exists in the roles field on the user object.

So what if a malicious user subscribes to the users publication with the following userId parameter:

Meteor.subscribe('users', {roles: ['admin']});

Uh oh. We’re passing an object to our subscription, which we’re passing directly into Roles.userIsInRole. userIsInRole happily accepts this object, assuming that it’s a user object pulled from the database, and confirms for us that 'admin' is indeed in the roles field of the object. Great!

Fixing It

The correct fix for this issue is to not pass in the ID of the user, but instead use this.userId within the server method. This ensures that the user can’t “spoof” the system into thinking they’re someone else.

There are other lessons to be learned here, too.

Always check your arguments! When accepting user provided arguments in methods or publication, always use Meteor’s check method to ensure that the argument you’re getting is of the type you expect.

Lastly, it’s very important to always be aware of what’s going on in any third party code you’re using. Without thoroughly reading the docs, it might not be immediately obvious that the userIsInRole method accepts either a String or an Object. Or, maybe it’s assumed that the package itself is checking its arguments. Never assume! Always check!

Finishing up, the correct publication looks like this:

Meteor.publish('users', function(){
    if(Roles.userIsInRole(this.userId, 'admin')){
        return Meteor.users.find({}, {fields: {...});
    }
});