Unable to use index patterns against indices with a __proto__ field

[legrego]

The elasticsearch-js client that Kibana uses for all communication to Elasticsearch has builtin protections to mitigate the risk of prototype pollution attacks. While this is useful in practice, it can have unintended consequences.

It appears that these protections are preventing index patterns from being used if there is an index mapping with a __proto__ field.

Reproduction steps

1. Create an index with a mapping containing a __proto__ field:

PUT my-test-index
{
  "mappings": {
    "properties": {
      "@timestamp": {
        "type": "date"
      },
      "__proto__": {
        "properties": {
          "field": {
            "type": "keyword",
            "ignore_above": 256
          }
        }
      }
    }
  }
}

2. Create an index pattern against my-test-index

index-pattern-error.mp4

---

I believe the solution is to turn off the protections provided by the elasticsearch-js client for the calls that are failing (see the disablePrototypePoisoningProtection option in the docs). It won't just be that simple though, as you'll need to ensure that you are not processing the field data in a dangerous way.

[elasticmachine]

Pinging @elastic/kibana-app-services (Team:AppServices)

[mattkime]

@legrego I'm thinking about how to approach this and it might be best to block __proto__ and similar fields from appearing (perhaps displaying an error). Ensuring that index pattern code handles the field correctly is easy - ensuring that its handled appropriately in all consuming applications seems quixotic. Further, its just not a responsibility we should be handing to our consuming applications.

I haven't seen anyone specifically need fields with such names. What do you think?

[legrego]

I haven't seen anyone specifically need fields with such names. What do you think?

I opened this issue because it was reported by an end-user, not because I noticed a theoretical issue. I agree it is esoteric, but I don't think we should be banning the use of these fields outright, especially in a foundational feature such as index patterns data views.

I'm thinking about how to approach this and it might be best to block proto and similar fields from appearing (perhaps displaying an error).

I agree that blocking __proto__ would be the safest option, but that artificially limits what our users can do, because of a language quirk.

I think we should ensure that the index pattern code can handle this field correctly, and perhaps add __proto__ as a field within our common FTR suites.

Further, its just not a responsibility we should be handing to our consuming applications.

I disagree. Index patterns are inherently comprised of user-supplied (read: untrusted) data, and it is everyone's responsibility to ensure they are handling this data in an appropriate way. This includes defending against prototype pollution attacks.

cc @elastic/kibana-security if anyone else has thoughts on this

[mattkime]

@legrego Those are good points but I could use some guidance in how to proceed.

In order to properly handle __proto__ as a field it seems that the first step would be for all consuming applications to have tests verifying that they properly support it. Perhaps we need a flag that allows unsafe field names until our apps are properly tested.

In the mean time, you don't get ANY fields if you have a __proto__ field. Blocking the field provides more functionality until we've verified that our consuming apps are safe.

Anyways, thats just off the top of my head. I'm open to other ideas on how to approach this.

[legrego]

In order to properly handle __proto__ as a field it seems that the first step would be for all consuming applications to have tests verifying that they properly support it. Perhaps we need a flag that allows unsafe field names until our apps are properly tested.

I like this idea. Could this be something that is determined by our apps at runtime? That way, apps could independently opt-in to this behavior once they've certified themselves, rather than waiting for the entire Kibana ecosystem to be ready.

In the mean time, you don't get ANY fields if you have a __proto__ field. Blocking the field provides more functionality until we've verified that our consuming apps are safe.

This could be problematic now that index patterns no longer cache fields, and instead load them on-demand. This means that the introduction of a __proto__ field into an existing index pattern would break all downstream consumers of the pattern. If an attacker managed to introduce this field mapping, then they could leverage this behavior to effectively disable anything relies on a particular index pattern.

[mattkime]

Could this be something that is determined by our apps at runtime?

Obviously anything is possible but I dislike the idea that index patterns would have different field lists based on which app is using them. That goes against the core mission of index patterns.

[mattkime]

I'm wondering if using a Map would avoid all of the __proto__ hassle.

[legrego]

Yes, Maps are a great way to work with untrusted data such as __proto__. You still have to be careful with what you do with the data once you take it out of the Map, but in general replacing the use of Objects with Maps is a good thing in terms of security 👍

[mshustov]

I'm wondering if using a Map would avoid all of the proto hassle.

Maps are not serialized/deserialized automatically with JSON methods, we would need a custom serialization/deserialization mechanism for both server and browser envs to support data pipeline Kibana browser --> Kibana server --> Elasticsearch server.

I believe the solution is to turn off the protections provided by the elasticsearch-js client for the calls that are failing (see the disablePrototypePoisoningProtection option in the docs). It won't just be that simple though, as you'll need to ensure that you are not processing the field data in a dangerous way.

@jportner could you confirm you are okay if Core disables prototype pollution mechanism for @elastic/elasticsearch client?
as @legrego noticed:

 I don’t think it’s reasonable to expect the ES client to defend against these types of attacks, because we will be working with user-supplied data —  we can’t assume that users won’t have a valid reason for having _proto_ in their payloads

Also, @jportner do you know if we have any prototype pollution protection mechanism on the browser side? In this case, a Kibana server response might be broken in the browser again.

[jportner]

@jportner could you confirm you are okay if Core disables prototype pollution mechanism for @elastic/elasticsearch client?
as @legrego noticed:

 I don’t think it’s reasonable to expect the ES client to defend against these types of attacks, because we will be working with user-supplied data —  we can’t assume that users won’t have a valid reason for having _proto_ in their payloads

I don't think Larry was suggesting that we use disablePrototypePoisoningProtection for all ES client calls. I think we need to expose the option for consumers like the data plugin to use it when necessary.

@mattkime correct me if I'm wrong -- all consumers of index patterns rely on the data plugin to actually load the index fields, right? If so, it sounds like that's the only consumer that would need to change the ES client option.

Also, @jportner do you know if we have any prototype pollution protection mechanism on the browser side? In this case, a Kibana server response might be broken in the browser again.

No, we don't have any "default" prototype pollution controls in the client code. It's difficult/impossible to implement broad protection against this while ensuring that existing code won't break. See also: Security best practices documentation

[mshustov]

I don't think Larry was suggesting that we use disablePrototypePoisoningProtection for all ES client calls. I think we need to expose the option for consumers like the data plugin to use it when necessary.

We can work it around by allowing plugins to create a custom client with disablePrototypePoisoningProtection: true. However, plugins deal with user-supplied data not only via data plugin but via the ES client as well. Ofc, we can be defensive and refactor plugins (as suggested above) on request.

[mattkime]

all consumers of index patterns rely on the data plugin to actually load the index fields, right? If so, it sounds like that's the only consumer that would need to change the ES client option.

correct

[legrego]

I don't think Larry was suggesting that we use disablePrototypePoisoningProtection for all ES client calls. I think we need to expose the option for consumers like the data plugin to use it when necessary.

@jportner actually I was proposing this. I expect that a majority of ES APIs return user-supplied data, to which we have no control. I think it's perfectly reasonable for us to support __proto__ as a piece of user-supplied data. The fact that this field is dangerous to our language du jour shouldn't mean that our users can't use it. Rather, I feel that it should serve as a reminder that most/all data in ES should be considered untrusted, and treated as such.

If we keep this as an option in the ES Client, then I feel the ES Client would need to know which calls contain user-supplied data, and which ones do not. The ES Client could filter out __proto__ and friends for the calls that do not return user-supplied data, but only if we are reasonably confident that the field shouldn't ever exist in the payload.

[mshustov]

@mattkime How quickly do you need to solve this problem? Should we set a deadline to decide on a preferred solution?

[mattkime]

I see it as a denial-of-service bug/exploit and would like to see it addressed soon. Would be nice to see it as part of 7.16 and perhaps backported to 7.15

[legrego]

@mattkime I tested today, and other fields such as toString are also problematic. While you can't pollute the global prototype using this, it's possible to corrupt the individual objects:

Index a document

POST test/_doc/1
{
  "toString": true
}

Create an index pattern

View in Discover