$populate security risk

[DominikTrenz]

Steps to reproduce

Following scenario:
I got two services : "comments" and "users".
Users is the private service which holds the user data and can only be accessed if authorized.
Comments is a public endpoint and serves datasets containing a ObjectRef to the user that created the comment.
Now i do a "comments?$populate[]=user"

Expected behavior

It should not give me the complete user dataset without being authorized as this user

Actual behavior

It does give me everything. How can i get it secure?

System configuration

"feathers-mongoose": "^5.1.0",

[longseespace]

You can always write a hook to prevent this however I agree that $populate should be disabled by default.

[daffl]

Agreed. I've been debating of disabling anything that is not covered by the official query syntax by default and having to explicitly whitelist the functionality you want.

We're planning major releases for the database adapters for feathersjs/feathers#608 so that might be a good time to do that.

[ekryski]

imho $populate should be a valid query param but it should trigger populate hooks that have been registered on the service. That way it's a bit more explicit what you are exposing.

Using the internal mongoose $populate is nice for convenience but is kind of hack legacy solution from before @eddyystop did so much awesome work on the common hooks.

[crobinson42]

I agree with @ekryski. I don't think disabling a query param is the best solution. However, creating a before/after populate callback per service sounds flexible and reasonable enough to address the security concerns.

[dortamiguel]

There is a hook that disables populate for the moment?

[daffl]

function(hook) {
  delete hook.params.query.$populate;
}

?

[dortamiguel]

it works, I added that in the find and get before in the app.hooks.js

[dortamiguel]

If you still want that functionality in the backend you can use this hook

// Disables mongoose $populate in the front
const { when } = require('feathers-hooks-common')

module.exports = () => {
	return when(
		hook => hook.params.provider,
		hook => {
			delete hook.params.query.$populate
			return hook
		}
	)
}

[davidchalifoux]

@daffl Just ran in to this problem. I want to use the populate feature, however using the discard()hook is not working to remove data such as passwords. What is your suggested workaround to process the data?

Edit: At the moment I'm just including the removal of the sensitive data in a hook that serializes it into JSON-API format. I don't understand why discard() wouldn't work though?

[daffl]

What I said in my previous comment. Just delete those properties manually in an after hook or use something like Lodash _.omit.