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

Router: Add option to check unknown payload for prototype pollution #82458

Closed
flash1293 opened this issue Nov 3, 2020 · 4 comments
Closed

Router: Add option to check unknown payload for prototype pollution #82458

flash1293 opened this issue Nov 3, 2020 · 4 comments
Labels
Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@flash1293
Copy link
Contributor

It's required to define a validation schema for each route added via core router, with an option to allow unknown keys. In theory it makes sense for all routes to use strict validation schemes. But that's not always possible (e.g. when piping through Elasticsearch dsl) - for these cases there's no protection at all at the moment for prototype pollution. It would be helpful 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, failing the validation when encountering them. It's somewhere between the security of an actual schema and an unknown object payload.

This could be used by TSVB: #78908

@flash1293 flash1293 added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label Nov 3, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

@pgayvallet
Copy link
Contributor

We did, at some point, some preliminary work to allow that kind of 'security' checks to be performed by default on all routes

private safetyPrechecks<T>(data: T, namespace?: string): T {
// We can add any pre-validation safety logic in here
return data;
}
private safetyPostchecks<T>(data: T, namespace?: string): T {
// We can add any post-validation safety logic in here
return data;
}

Even if no-op for now, these validations are enabled by default, and must manually be opted out, via

  router.post(
    {
      path: '/somewhere',
      validate: {
        body: schema.object(
          {},
          { unknowns: 'allow' }
        ),
        unsafe: {
          body: true,
        },
      },
    },

We should add prototype pollution guard here.

cc @elastic/kibana-security

@watson
Copy link
Contributor

watson commented Nov 4, 2020

This is a possible duplicate of #49608 and #49609

@pgayvallet
Copy link
Contributor

It sure is a dupe of #49609. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

No branches or pull requests

4 participants