Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Meteor.uuid is deprecated and untrusted on the client side #108

Open
MechJosh0 opened this issue Jun 1, 2016 · 5 comments
Open

Meteor.uuid is deprecated and untrusted on the client side #108

MechJosh0 opened this issue Jun 1, 2016 · 5 comments

Comments

@MechJosh0
Copy link
Contributor

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
Copy link

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
Copy link
Contributor

xcv58 commented Jun 28, 2016

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
Copy link
Contributor

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
Copy link
Contributor

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

markshust added a commit to markshust/mantra-sample-blog-app that referenced this issue Jul 13, 2016
markshust added a commit to markshust/mantra-sample-blog-app that referenced this issue Jul 13, 2016
@markshust
Copy link
Contributor

markshust commented Jul 13, 2016

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().

#115

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants