Meteor.uuid is deprecated and untrusted on the client side

[MechJosh0]

On the client side, when we create a new post we use the Meteor.uuid() function to create a random string for the post _id, this is sent to the method and inserted into the database as perfectly fine to use as long as it passes check(_id, String).

There are two problems with this:

1. Meteor.uuid() is deprecated.
2. Open to abuse. There is no validation on the server side to check that the _id being passed is truly a random _id therefore a user could pass anything they wish. See my post on the demo website with an _id of lolcats.

A breakdown on how someone who cannot see the repo could work this out for those wondering: On the client console you can call Meteor.connection._methodHandlers to see a list of available methods. You can see near the bottom posts.create: e(e,t,r), in chrome you can right click this and "Show function definition", now we can see the method function itself which is (after unminified):

methods({
    "posts.create": function() {
        function e(e, t, r) {
            n.check(e, String), n.check(t, String), n.check(r, String);
            var s = new Date,
                u = {
                    _id: e,
                    title: t,
                    content: r,
                    createdAt: s,
                    saving: !0
                };
            o.Posts.insert(u)
        }
        return e
    }()
})

Now we know what is expected to be passed through and how each variable is being checked. So if we wanted, we could now do: Meteor.call('posts.create', 'lolcats', 'My Title', 'The Content') and server will accept this.

Now this isn't much of a security issue, this only opens the ability for a troll to have some fun. However, a personal project I'm fine with, a business website I'd like to close this. One solution would be to create the Meteor.uuid() on the server side however we would lose the method stub ability, I'm sure there are other solutions but this is all I can think of at the moment.

[macrozone]

I do not understand why the id is passed anyway.

Doesn't Collection.insert generate a id on its own if not passed? And this should even work if happens both in a client-stub and the server-method?

[xcv58]

The id of two sides should be the same. If you use the id generated by insert. Probably it's not same as client stub.

[markshust]

The id is passed in for latency compensation.

The solution is pretty simple, just need to create a custom check function for uuid, to verify it is correct. This would prevent passing in invalid uuids, and make the latency compensation still work.

something like this:

check(_id, Uuid);

[markshust]

FYI, the Meteor.uuid() call should most likely be replaced by Random.id() https://github.com/meteor/meteor/tree/master/packages/random

[markshust]

Submitted a pr for this fix.

A uuid validator is pretty much a check for a string that is 17 characters -- there really isn't a necessity to check for anything further, as it's all randomized anyways. You can see how this is created with a custom matcher at https://github.com/markoshust/mantra-sample-blog-app/blob/ef35a2fac1eba766eab08959b315c5f4636b38e7/lib/match.js

Then, just import that matcher and run check against it, as so https://github.com/markoshust/mantra-sample-blog-app/blob/0ae9cc607642ca99e76f6f0956a0f07dc60bf800/server/publications/posts.js

I'm not 100% sure this is really needed on the client... most likely just needed on server, but updated client scripts to keep things consistent.

I also updated Meteor.uuid() to Random.id().

https://github.com/mantrajs/mantra-sample-blog-app/pull/115