Take our Allow & Deny Security Challenge


#1

Allow & Deny vs Methods is one of the big debates within the Meteor community. Personally I tend to fall more on the side of Methods, simply because Allow & Deny is very hard to get right.

To illustrate this, I decided to hold a simple coding challenge:

https://www.discovermeteor.com/blog/allow-deny-security-challenge/

Fork the pad, post links to your answers here, and I’ll try to go through them to see if I can find any security holes ; )


Meteor allow/deny vulnerability disclosure
#2

Alright. I’ve updated my answer again. This should really be NSA proof!

Messages.allow 
  update: (userId, doc, fields, modifier) ->
    true
    # change this!
Messages.deny
  update: (userId, doc, fields, modifier) ->
    return true if !userId or 'createdAt' in fields or 'userId' in fields
    return true if modifier.$addToSet?.likes and doc.likes.indexOf(userId) != -1
    if 'likesCount' in fields
      return true if !modifier.$addToSet?.likes
      return true if !modifier.$inc?.likesCount
      return true if modifier.$inc.likesCount != 1
    return true if userId != doc.userId and 'body' in fields
    return false

#3

I have a more “refined” opinion on this topic now, I wonder what you think:

  • Allow/Deny is fine for insert, remove, and a hypothetical read; there’s only one concern that you might want computed properties on create, but collection hooks take care of that.
  • It is not fine for update

Thoughts?


#4

Can you create a MeteorPad with your answer?


#5

It can be tricky for insert as well, depending on what you want to do. Unless you combine it with collection hooks, that can work too.

Another rule I came up with: allow/deny is ok when you’re updating your own documents, but not a good idea when you want to give users permission to change something on other user’s documents.


#6

Sure can. Here:
http://meteorpad.com/pad/krqwC2tFfcb2R2xdg/Chatroom%20Security%20Challenge


#7

You still need to be careful about, for example, a user reassigning their document to some other user by modifying the user ID foreign key.


#8

Sorry, not secure :frowning: I was able to set likesCount to any value.


#9

Can you try again with the same meteorpad?


#10

Did we kill meteorpad? Takes forever for the server instance to load up and then once it does, it doesn’t update when I save my changes…


#11

My submission (also posted in comments on discover meteor post): http://meteorpad.com/pad/NEv9y3WsMgugHmBwg/copleykj’s%20Chatroom%20Security%20Challenge

I modified this a bit to use the socialize:likeable package which I authored but it still does the same basic thing and I believe satisfies all of the requirements.


#12

Here’s my submission.

http://meteorpad.com/pad/uSrCGEWX87YiRBtax/Copy%20of%20Chatroom%20Security%20Challenge


#13

@khamoud Sorry, not secure: Messages.update(Messages.findOne()._id, {$set: {body: "foo", likesCount: 9999}})


#14

Man. I can’t believe I missed that. It’s pretty obvious now that I think about my implementation (on mobile can’t see it). I’m partial to meteor methods for stuff like this for that exact reason.


#15

Results: https://www.discovermeteor.com/blog/allow-deny-challenge-results/


#16

This is probably my favorite post ever written about Meteor. Awesome.


#17

That means a lot! Glad you enjoyed it :slight_smile: