Is the way the Firebase database quickstart handles counts secure?

Question

I want to create an increment field for article likes.

I am referring to this link: https://firebase.google.com/docs/database/android/save-data#save_data_as_transactions

In the example there is code for increment field:

if (p.stars.containsKey(getUid())) {
    // Unstar the post and remove self from stars
    p.starCount = p.starCount - 1;
    p.stars.remove(getUid());
} else {
    // Star the post and add self to stars
    p.starCount = p.starCount + 1;
    p.stars.put(getUid(), true);
}

But how can I be sure if the user already liked/unliked the article?

In the example, user (hacker) might as well clear whole stars Map like this and it will save anyway:

p.stars = new HashMap<>();

and it will ruin the logic for other users who were already liked it.

I do not even think you can make rules for this, especially for "decrease count" action.

Any help, suggestions?

how are you saving this info in your database? – adolfosrs Jun 21 '16 at 20:52 — adolfosrs, Jun 21 '16 at 20:52

Frank van Puffelen · Accepted Answer · 2017-07-24T19:52:45.477

30

The security rules can do a few things:

ensure that a user can only add/remove their own uid to the stars node
```
"stars": {
  "$uid": {
    ".write": "$uid == auth.uid"
  }
}
```
ensure that a user can only change the starCount when they are adding their own uid to the stars node or removing it from there
ensure that the user can only increase/decrease starCount by 1

Even with these, it might indeed still be tricky to have a security rule that ensures that the starCount is equal to the number of uids in the stars node. I encourage you to try it though, and share your result.

The way I've seen most developers deal with this though is:

do the start counting on the client (if the size of the stars node is not too large, this is reasonable).
have a trusted process running on a server that aggregates the stars into starCount. It could use child_added/child_removed events for incrementing/decrementing.

Update: with working example

I wrote up a working example of a voting system. The data structure is:

votes: {
  uid1: true,
  uid2: true,
},
voteCount: 2

When a user votes, the app sends a multi-location update:

{
  "/votes/uid3": true,
  "voteCount": 3
}

And then to remove their vote:

{
  "/votes/uid3": null,
  "voteCount": 2
}

This means the app needs to explicitly read the current value for voteCount, with:

function vote(auth) {
  ref.child('voteCount').once('value', function(voteCount) {
    var updates = {};
    updates['votes/'+auth.uid] = true;
    updates.voteCount = voteCount.val() + 1;
    ref.update(updates);
  });  
}

It's essentially a multi-location transaction, but then built in app code and security rules instead of the Firebase SDK and server itself.

The security rules do a few things:

ensure that the voteCount can only go up or down by 1
ensure that a user can only add/remove their own vote
ensure that a count increase is accompanied by a vote
ensure that a count decrease is accompanied by a "unvote"
ensure that a vote is accompanied by a count increase

Note that the rules don't:

ensure that an "unvote" is accompanied by a count decrease (can be done with a .write rule)
retry failed votes/unvotes (to handle concurrent voting/unvoting)

The rules:

"votes": {
    "$uid": {
      ".write": "auth.uid == $uid",
      ".validate": "(!data.exists() && newData.val() == true &&
                      newData.parent().parent().child('voteCount').val() == data.parent().parent().child('voteCount').val() + 1
                    )"
    }
},
"voteCount": {
    ".validate": "(newData.val() == data.val() + 1 && 
                   newData.parent().child('votes').child(auth.uid).val() == true && 
                   !data.parent().child('votes').child(auth.uid).exists()
                  ) || 
                  (newData.val() == data.val() - 1 && 
                   !newData.parent().child('votes').child(auth.uid).exists() && 
                   data.parent().child('votes').child(auth.uid).val() == true
                  )",
    ".write": "auth != null"
}

jsbin with some code to test this: http://jsbin.com/yaxexe/edit?js,console

edited Jul 24 '17 at 19:52

answered Jun 22 '16 at 00:13

Frank van Puffelen

565,676
79
828
807

Thanks for the ideas, looks like most of them would work in my situation. – Zigmārs Dzērve Jun 22 '16 at 05:41
Darn... I was hoping you'd take a stab at securing it through rules. :-) I'd really like to see at some point how close one could get with that. – Frank van Puffelen Jun 22 '16 at 16:28
1

Firebase rules are pretty limiting. Especially when all the upper "write" rules overwrite any lower ones. Your rule has a problem, that I can't add a write permission for upper "$article_id" column, if I want this lower "$uid == auth.uid" to work. so basically no one will ever have permission to create a new entry in upper "articles". I was so hyped about firebase, but these rules are so limiting you can't even solve simple things with them. – Zigmārs Dzērve Jun 22 '16 at 22:05
FIrebase's security rules are "somewhat unconventional". But if you take the time to wrap your head around them, these rules are surprisingly powerful. We've seen people implement game transactions (with two-phase commit like interaction) in them, as well as many more common security and validation systems. – Frank van Puffelen Jun 23 '16 at 04:13
1

Then please elaborate how would you suggest getting `".write": "$uid == auth.uid"` to work on "stars" level. Because I get this error un parent level: Transaction at /article-favorites/-KK9xFyA8skOvSLhaHWq failed: DatabaseError: Permission denied But if I add write rule to "article-favorites" then that uid check that is deeper will be ignored. Am I missing something? Can I use ".validate" for this purpose or ? – Zigmārs Dzērve Jun 23 '16 at 08:59
If I move "$uid == auth.uid" to validate and add a write rule for the parent, it's all good when an article has only 1 like. But when another user tries to like an article, validation will fail, because Map also contains an uid that isn't equal with currently logged in users. I am starting to lose hope. – Zigmārs Dzērve Jun 23 '16 at 10:16
@MJQZ1347 First of all i just made star counting a client functionality, on every open to count all entries, which sucks. Second of all since January 1st, 2017 my app is 100% migrated away from firebase. Just wasnt for me. – Zigmārs Dzērve Jan 13 '17 at 06:46
@ZigmārsDzērve still curious, where did you move to? Some lacking features of firebase are also making me rethink my decision. – MJQZ1347 Jan 13 '17 at 10:33
@MJQZ1347 I made a commitment and spent 2 or 3 months writing server-side myself using language and tools I want. Of course, it was a lot more work at start, but now I am not limited to anything and have all options available. I see firebase as a prototyping tool to create simple MVP products fast, but I 100% don't recommend it for production apps because you can't really create anything complicated with it. – Zigmārs Dzērve Jan 13 '17 at 11:19
is there example of this using the angularFire2 library? – alltej Feb 26 '17 at 13:53
This is an awesome way to keep track of counts on Firebase. Thank you! – Dreamingwhale Jun 07 '17 at 12:49

Is the way the Firebase database quickstart handles counts secure?

1 Answers1

Update: with working example

Linked

Related