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

Enable prototype pollution protection in TSVB #78908

Closed
flash1293 opened this issue Sep 30, 2020 · 4 comments · Fixed by #85952
Closed

Enable prototype pollution protection in TSVB #78908

flash1293 opened this issue Sep 30, 2020 · 4 comments · Fixed by #85952
Assignees
Labels
Feature:TSVB TSVB (Time Series Visual Builder) Team:Visualizations Visualization editors, elastic-charts and infrastructure

Comments

@flash1293
Copy link
Contributor

Currently the TSVB endpoint in

visPayloadSchema.validate(request.body);
is only doing a soft validation based on the provided schema because there are legacy configuration objects currently incompatible with the schema.

To put some protection in place, the same logic as used in core should be applied, failing the request if some known malicious keys are specified somewhere in the request object:

export function validateObject(obj: any) {

We should still strive to complete the strict schema to cover all possible configurations in the long term.

Tasks:

  • Export the validateObject helper method from core so it's usable in plugins as well
  • Use it for the TSVB endpoint along with the soft validation
@flash1293 flash1293 added Feature:TSVB TSVB (Time Series Visual Builder) Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Sep 30, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@flash1293
Copy link
Contributor Author

cc @timroes

@timroes
Copy link
Contributor

timroes commented Sep 30, 2020

@joshdover do you think it would make sense to export this method from core, so every HTTP service could use it instead of copying that code over?

@flash1293
Copy link
Contributor Author

flash1293 commented Sep 30, 2020

I guess in theory we want all routes to use strict validation schemes. But that's not always possible (e.g. when piping through Elasticsearch dsl) - for these cases one approach would be to introduce a new option as part of @kbn/config-schema:
schema.object({}, { unknowns: 'allowSafe' }) This would do the recursive check for prototype pollution keys

DianaDerevyankina added a commit to DianaDerevyankina/kibana that referenced this issue Dec 15, 2020
alexwizp pushed a commit that referenced this issue Dec 31, 2020
* Enable prototype pollution protection in TSVB

Closes #78908

* Update Dock API Changes

* Replace logging failed in validateObject validation with 400 error

* Move validateObject to kbn-std package and add a description

* Update Doc API Changes

* Rename validateObject function to ensureNoUnsafeProperties

* Rename other validateObject occurrences

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
alexwizp pushed a commit to alexwizp/kibana that referenced this issue Dec 31, 2020
* Enable prototype pollution protection in TSVB

Closes elastic#78908

* Update Dock API Changes

* Replace logging failed in validateObject validation with 400 error

* Move validateObject to kbn-std package and add a description

* Update Doc API Changes

* Rename validateObject function to ensureNoUnsafeProperties

* Rename other validateObject occurrences

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
alexwizp added a commit that referenced this issue Dec 31, 2020
* Enable prototype pollution protection in TSVB

Closes #78908

* Update Dock API Changes

* Replace logging failed in validateObject validation with 400 error

* Move validateObject to kbn-std package and add a description

* Update Doc API Changes

* Rename validateObject function to ensureNoUnsafeProperties

* Rename other validateObject occurrences

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Diana Derevyankina <54894989+DziyanaDzeraviankina@users.noreply.github.com>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:TSVB TSVB (Time Series Visual Builder) Team:Visualizations Visualization editors, elastic-charts and infrastructure
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants