Repository navigation
adapter commons whitelist skips filter items? #1971
Description
Steps to reproduce
First, not sure where adapter-commons is in 4.5.3.
We have a query in the client (using the Feathers client):
const item = await myService.get(this.id, {
query: {
$populate: ['user']
}
});
What's notable is myService does NOT whitelist $populate. I've confirmed at no point is it there.
I'm confused by this code in cleanQuery:
_.each(query, (value, key) => {
if (key[0] === '$') {
if (filters[key] !== undefined) {
return;
}
if (!operators.includes(key)) {
throw new BadRequest(`Invalid query parameter ${key}`, query);
}
}
The code thinks my $populate is part of the filters, so it sees that we have a filter with this operator and lets it through! What's the point of the operator check if things like $populate are not considered?
What does it mean for an operator to be part of the query vs the filter? I had trouble finding docs that explained it and this behavior.
This definitely worked at some point (where it would error out in this scenario).
Expected behavior
I would expect $populate to error in this case because I have not whitelisted it.
Actual behavior
The query succeeds and does the $populate
System configuration
Module versions (especially the part that's not working):
feathers-mongoose 8.3.0
adapter-commons 4.4.3 (because feathers-mongoose 8.3.0 specifies ^4.4.1)
feathersjs 4.5.3
NodeJS version:
12.16.3
Activity
J3m5 commentedon May 28, 2020
I think that it's added by default.
https://github.com/feathersjs-ecosystem/feathers-mongoose/blob/56202362c7206149d7c8b1a84f89413b6aa8d283/lib/service.js#L19
jnardone commentedon May 29, 2020
If so then what's the point of the whitelist? We want to disallow $populate from the client except where we explicitly allow it in feathers-mongoose, but not clear when that regressed (it used to work as expected...)
J3m5 commentedon May 29, 2020
It seems to lead to that change.
daffl commentedon May 29, 2020
$populateis officially supported by thefeathers-mongooseadapter (documented at https://github.com/feathersjs-ecosystem/feathers-mongoose#populate). If you want to disable it you can pass your own filters in the service options atsrc/services/<name>/<name>.service.js:jnardone commentedon May 29, 2020
That page explicitly says
Important: $populate has to be whitelisted explicitly since it can expose protected fields in sub-documents (like the user password) which have to be removed manually.The service being called DOES NOT WHITELIST $populate yet it is allowed. This feels like a bug - shouldn't the whitelist check happen before the filter check?
daffl commentedon Jun 1, 2020
That is correct and indeed does not seem to be the case at the moment.
eugyev commentedon Mar 23, 2021
this issue is still open as far as I can tell and represents a pretty big security flaw - especially since it seems like you can populate a userId and get that user's password potentially. Fix would be to remove populate from the default filters list in section in lib/service.js. also should probably add filters tot he list of allowed options in typescript to go to the service creation?
daffl commentedon Apr 8, 2021
The
$populateissue is fixed now in the latest version offeathers-mongoose.