Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Leaking private/protected data #252

Closed
smirea opened this issue Feb 13, 2016 · 2 comments
Closed

Leaking private/protected data #252

smirea opened this issue Feb 13, 2016 · 2 comments
Labels

Comments

@smirea
Copy link
Contributor

smirea commented Feb 13, 2016

say you have an User model:

const User = mongoose.model('User', new mongoose.Schema({
    name: String,
    password: String,
}));

normally you want to never expose the password, under any circumstances, so you'd normally do:

restify.serve(router, User, {
    private: ['password'],
})

Now this works with hitting any endpoint:
GET /User does not show the fields
GET /User/some-id also does not show the password

HOWEVER:

GET /User?distinct=password shows ALL passwords for ALL users in the database ...

This is a huge security concern

@smirea
Copy link
Contributor Author

smirea commented Feb 13, 2016

if anyone stumbles upon this in the meantime, I've fixed it for myself using a preMiddleware:

preMiddleware: (req, res, next) => {
    if (!req.query.distinct) return next();
    const schema = req.erm && req.erm.model && req.erm.model.schema;
    const prop = schema && schema.path && schema.path(req.query.distinct);
    if (!prop || !prop.options || prop.options.access != 'private') return next();
    res.status(400);
    res.json({error: `Invalid propert ${req.query.distinct}`})
}

This requires that you define access: 'private' in your mongoose schemas:

const User = mongoose.model('User', new mongoose.Schema({
    name: String,
    password: {type: String, access: 'private'},
}));

but this has the nice benefit that it filters them out in the normal aforementioned cases while also disabling the distinct query with the same syntax

@Zertz Zertz added the security label Feb 13, 2016
@Zertz
Copy link
Collaborator

Zertz commented Feb 15, 2016

Confirmed, thank you. I wrote some tests and have a fix, it just needs some extra polish. I should be able to get something out in the next couple of days.

Zertz pushed a commit that referenced this issue Feb 16, 2016
This will filter out distinct queries containing a private or protected
field. The reason for filtering post-query is to mimic behavior where
the field did not exist, which is to return an empty array.

@smirea Can you test the patch on your end?

Closes #252
Zertz pushed a commit that referenced this issue Feb 19, 2016
This will filter out distinct queries containing a private or protected
field. The reason for filtering post-query is to mimic behavior where
the field did not exist, which is to return an empty array.

@smirea Can you test the patch on your end?

Closes #252
Zertz pushed a commit that referenced this issue Feb 19, 2016
This will filter out distinct queries containing a private or protected field. As suggested, distinct fields are now verified before querying, thus saving a database call and fields that are filtered out return an empty array, as if the field did not exist.

Closes #252
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants