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
Machine leaks API information in production #5626
Comments
@alxndrsn Thanks for posting! We'll take a look as soon as possible. In the mean time, there are a few ways you can help speed things along:
Please remember: never post in a public forum if you believe you've found a genuine security vulnerability. Instead, disclose it responsibly. For help with questions about Sails, click here. |
@alxndrsn this totally make sense, maybe some kind of function that would run in this scenario and return the desired response body (leaving the headers & status code the same). If anyone else looking at this issue has anything to add, open to more suggestions/feedback! |
Although the framework has great power with its automation, you can provide a workaround with your own controller I presume. |
@crh3675 this is true, but inconvenient if it has to be done in many controllers simultaneously. |
Policies are processed before controllers, perhaps in there? You can apply a global policy across all controllers which might be another temporary solution |
As far as I could work out, you can only have one policy per controller. Also sounds a bit hacky... maybe more intuitive would be an error-handling middleware which filters out the messages I don't like. |
You can have multiple policies per controller - we do it all the time- just use an array instead. Or set a wildcard catchall.
Not that with multiple policies, you need to enfore the usage of next in the polict
|
@crh3675 this is true. I got confused, because I've tried in the past to enforce policies like "is admin OR manager", but this doesn't seem simple to achieve via policies alone - the combined policies such as
are a conjunction rather than disjunction, so I've ended up with policies like |
@raqem I think there are workarounds implied, but not clearly defined. Options seem to be:
I'd guess that 2 is the less risky option, but I can't confirm that it either works or is reliable. I'll share info here when I've implemented a workaround. |
@crh3675 going back to your original point:
As there's no setting for |
I guess it would be easier if the machine actually threw an error that was catchable. I understand a bit more after looking into the machine modularization. Looks like you might be able to create a custom |
I am finding a solution for this too, These messages can be leak our api params to outside. |
Super simple fix in in responses/badRequest
|
@crh3675 neat. Do you mean in |
When you create a new Sails application you should have a folder under api/responses of which you can change |
No, sails will not create response folder automatically. You will need to create the folder also the customResponse file by yourself |
So it seems that the solution is to copy |
Yeah, the solution I choose like this:
|
In production, when required param(s) are omitted from e.g. a POST request, machine's
E_MISSING_OR_INVALID_PARAMS
response will leak information about the API.E.g.:
This error message is generated in the
machine-as-action
module, and there does not appear to be an option to disable this.These detailed messages are very helpful in development, and likely if building a production API to be consumed by third parties.
However, for some projects they represent an unacceptable level of data leakage.
It would be great if there were an option to disable these detailed error messages in production, and instead return a straight
400
error with content simplyBad Request
(text/plain
).The text was updated successfully, but these errors were encountered: