BUG: Firebase queue security rule

[andlcool]

Version info

Firebase-admin: 4.1.0

Firebase: 3.6.1

Firebase Queue: 1.6.0

Node.js: v7.0.0

Other (e.g. operating system) (if applicable): MacOS Sierra 10.12.1

Test case

Steps to reproduce

Step 1: with this Firebase queue security rules

{
   "rules":{
      "user_reminder_queue":{
         "tasks":{
            ".read":"auth.uid === 'my-service-worker'",
            ".write":"auth !== null",
            ".indexOn":"_state",
            "$taskId":{
               ".validate":"newData.hasChildren(['pushKey', 'message', 'reminder_date']) || (auth.uid === 'my-service-worker' && newData.hasChildren(['_state', '_state_changed', '_progress']))",
               "_state":{
                  ".validate":"newData.isString()"
               },
               "_state_changed":{
                  ".validate":"newData.isNumber() && (newData.val() === now || data.val() === newData.val())"
               },
               "_owner":{
                  ".validate":"newData.isString()"
               },
               "_progress":{
                  ".validate":"newData.isNumber() && newData.val() >= 0 && newData.val() <= 100"
               },
               "_error_details":{
                  "error":{
                     ".validate":"newData.isString()"
                  },
                  "error_stack":{
                     ".validate":"newData.isString()"
                  },
                  "previous_state":{
                     ".validate":"newData.isString()"
                  },
                  "attempts":{
                     ".validate":"newData.isNumber() && newData.val() > 0"
                  },
                  "$other":{
                     ".validate":false
                  }
               },
               "_id":{
                  ".validate":"newData.isString()"
               },
               "pushKey":{
                  ".validate":"newData.isString()"
               },
               "message":{
                  ".validate":"newData.isString() && newData.val().length <= 1000"
               }
            }
         },
         "specs":{
            ".read":"auth.uid === 'my-service-worker'",
            ".write":"auth.uid ==='my-service-worker'",
            "$specId":{
               ".validate":"newData.hasChild('in_progress_state')",
               "start_state":{
                  ".validate":"newData.isString()"
               },
               "in_progress_state":{
                  ".validate":"newData.isString()"
               },
               "finished_state":{
                  ".validate":"newData.isString()"
               },
               "error_state":{
                  ".validate":"newData.isString()"
               },
               "timeout":{
                  ".validate":"newData.isNumber() && newData.val() > 0"
               },
               "retries":{
                  ".validate":"newData.isNumber() && newData.val() >= 0"
               },
               "$other":{
                  ".validate":false
               }
            }
         }
      }
   }
}

Step 2:

//this pushes a $taskId into my task node. This is the recommended way and yields correct results
queueRef.child('tasks').push({
    'message': 'happy 22: )',
    'pushKey': '-Xsdf24rasdfsdfG',
    'reminder_date' : '12-12-2017',
    'hello': 'world'
});

Step 3: This overwrite the user_reminder_queue > tasks > data (This behaviour is undesirable)

//this overwrite all my data in my tasks node. I want to prevent malicious hackers from performing this overwrite. How do I prevent is through the recommended security rules
queueRef.child('tasks').set({
    'taskIdabc': {
        'message': 'happy 22: )',
        'pushKey': '-Xsdf24rasdfsdfG',
        'reminder_date': '12-12-2017',
        'hello': 'world'
    }
});

Expected behavior

With the recommended firebase queue security rules, see https://github.com/firebase/firebase-queue/blob/master/docs/guide.md#queue-security, a set operation should not overwrite existing tasks in the queue.

Actual behavior

Existing tasks in the queue > tasks can be overwritten.

[katowulf]

The push key is actually designed to be unguessable. So I think this is far fetched as a security concern and an edge case you are 2^120 unlikely to encounter. But adding a security rule to check for this certainly won't do any harm either!

[andlcool]

@katowulf thanks for the reply. Unless i am mistaken, by using the set method, the push key is not needed to overwrite all the items in tasks.

[katowulf]

So the problem here being that it's at the parent level instead of the individual task level. Agreed.

It looks like, in the example rules, it should be this:

  "rules": {
    "queue": {
      "tasks": {
        ".read": "auth.canProcessTasks",
        ".indexOn": "_state",
        "$taskId": {
             ".write": "auth.canAddTasks && !data.exists() || auth.canProcessTasks",
             .... 
   }}}}

[andlcool]

How could i have missed that! The security issue coupled with this rule could then be mitigated by the push key being designed to be unguessable.

Thanks so much for the solution. 👍