Any security risk in using $set directly in a method?

[csiszi]

Hi,

I have a method which can update an item in the Items collection. There are 2 fields I consider important: companyId and apartmentIds. If these are restricted, the user can only “hack” his own items.

Is there a mongodb operator I’m not thinking about which can cause trouble if my method looks like this:

run({ _id, companyId, apartmentIds, ...doc }) {
  if (Meteor.isServer) {
    doc.companyId = this.companyId // it's provided by a validated method mixin
    doc.apartmentIds = restrictApartmentIds(companyId, apartmentIds) // remove the apIds not owned by this companyId
    return Items.update({ _id }, { $set: doc })
  }
}

[coagmano]

Looks good at first glance.
Are you validating doc? I wonder if anything could slip through there?

[csiszi]

Yes, it’s more or less the simpl-schema of the collection:

schema
    .omit('createdAt', 'updatedAt')
    .extend({ _id: String })

[znewsham]

Since you accept those ids from the client, they can pass in any values, they could change the value of any items company id, not sure if that is the goal or not?

[csiszi]

Yes, I have a lookup for the item owner before the update as well, just simplified the method.

Just curious, if the method has no validation, can the companyId and apartmentIds be modified somehow if the doc doesn’t have these top-level fields/keys?

[gabrielbalsa]

Code works on server, under Meteor.isServer there’s no easy way to modify anything. That said, it depends on your validated method mixin.

As long as you depend on an userId passed through the auth process, there wouldn’t be much to worry about