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

StackOverflow crash - large regex produced by Discover filter not limited by index.regex_max_length #82923

Closed
gplechuck opened this issue Jan 24, 2022 · 8 comments · Fixed by #84592 or #84624
Assignees
Labels
:Analytics/Aggregations Aggregations >bug :Search/Search Search-related issues that do not fall into other categories Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo)

Comments

@gplechuck
Copy link

Elasticsearch version (bin/elasticsearch --version): 7.10.2

Plugins installed: [repository-s3]

JVM version (java -version):
openjdk version "15.0.1" 2020-10-20
OpenJDK Runtime Environment AdoptOpenJDK (build 15.0.1+9)
OpenJDK 64-Bit Server VM AdoptOpenJDK (build 15.0.1+9, mixed mode, sharing)

OS version (uname -a if on a Unix-like system):
Debian Stretch

Description of the problem including expected versus actual behavior:
It seems to be possible to crash several Elasticsearch nodes by providing a very large string when attempting to filter on a field value (StackOverflow related to regexp processing). When filtering on a field value, a query containing a 'suggestions' aggregation is sent to the cluster in the background before the filter is saved, in order to populate an autocomplete drop down. This aggregation includes a regex which is constructed from taking the large string and suffixing with a ".*" . The resulting regexp does not seem to respect the default index.max_regex_length limit of 1000 - the query is submitted causing an instant crash of nodes.

Caused by: java.lang.StackOverflowError at org.apache.lucene.util.automaton.RegExp.parseSimpleExp(RegExp.java:1209) ~[lucene-core-8.7.0.jar:8.7.0 2dc63e901c60cda27ef3b744bc554f1481b3b067 - atrisharma - 2020-10-29 19:35:28]

This might seem like an unlikely scenario, but we have seen end users of Kibana trigger this bug several times (accidentally pasting copied output from a data table visualization into the 'value' field when filtering further for example)

Steps to reproduce:

  1. Browse an index in the Kibana Discover pane
  2. Click 'Add Filter'
  3. In 'Edit Filter' - select a field, choose 'is' , paste a large string into the value box

For a test I pasted 50k chars - I haven't done extensive testing to find where the crashing occurs.

Provide logs (if relevant):
Full crash logs (fatal-error.log) attached.
fatal-error.log

Example suggestions aggregation containing offending regex attached.
suggestions-query.txt

Unfortunately, disabling expensive queries is not an option for us as we have users which depend on regex and script query functionality. If you have any suggestions on how to best prevent this issue from re-occurring, that would be great.

Any questions, shout.

Cheers

@gplechuck gplechuck added >bug needs:triage Requires assignment of a team area label labels Jan 24, 2022
@gplechuck gplechuck changed the title StackOverflow crash - large regex in Discover filter not limited by index.regex_max_length StackOverflow crash - large regex produced by Discover filter not limited by index.regex_max_length Jan 24, 2022
@romseygeek romseygeek added the :Search/Search Search-related issues that do not fall into other categories label Jan 25, 2022
@elasticmachine elasticmachine added the Team:Search Meta label for search team label Jan 25, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (Team:Search)

@romseygeek romseygeek added :Analytics/Aggregations Aggregations and removed Team:Search Meta label for search team needs:triage Requires assignment of a team area label labels Jan 25, 2022
@elasticmachine elasticmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Jan 25, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo (Team:Analytics)

@nik9000
Copy link
Member

nik9000 commented Mar 2, 2022

I'm not a fan of the regex ctor being recursive. But it is..... Our standard defense against stuff like this is index.max_regex_length. We just need to invoke it here.

@nik9000 nik9000 self-assigned this Mar 2, 2022
@nik9000
Copy link
Member

nik9000 commented Mar 2, 2022

I had a look at this. Right now we build the Regexp when we're parsing the request. But the setting is most naturally available on the data node when we're building the query. I think that's not too bad.

nik9000 added a commit to nik9000/elasticsearch that referenced this issue Mar 2, 2022
In order to fix an error where large regexes in `include` or
`exclude` fields of the `terms` agg crash the node (elastic#82923) I'd like to
centralize construction of the `RegExp` so we can test it for
large-ness in one spot. The trouble is, there are half a dozen ctors for
`IncludeExclude` and some take `String` and some take `RegExp` and some
take a sets of `String` and some take sets of `BytesRef`. It's all very
convenient for client code, but confusing to deal with. This removes all
but two of the ctors for `IncludeExclude` and mostly standardizes on one
that has:
```
String includeRe, String excludeRe, Set<BytesRef> includePrecise, Set<BytesRef> excludePecise
```

Now I can fix elastic#82923 in a fairly simple follow up.
nik9000 added a commit that referenced this issue Mar 3, 2022
In order to fix an error where large regexes in `include` or
`exclude` fields of the `terms` agg crash the node (#82923) I'd like to
centralize construction of the `RegExp` so we can test it for
large-ness in one spot. The trouble is, there are half a dozen ctors for
`IncludeExclude` and some take `String` and some take `RegExp` and some
take a sets of `String` and some take sets of `BytesRef`. It's all very
convenient for client code, but confusing to deal with. This removes all
but two of the ctors for `IncludeExclude` and mostly standardizes on one
that has:
```
String includeRe, String excludeRe, Set<BytesRef> includePrecise, Set<BytesRef> excludePecise
```

Now I can fix #82923 in a fairly simple follow up.
@nik9000 nik9000 reopened this Mar 3, 2022
@nik9000
Copy link
Member

nik9000 commented Mar 3, 2022

#84592 does not close this issue.

@dblock
Copy link

dblock commented Apr 7, 2022

We tracked down what looks like this exact problem in OpenSearch, here's a fix that enforces the limit: opensearch-project/OpenSearch#2810.

@nik9000
Copy link
Member

nik9000 commented Apr 7, 2022 via email

@dblock
Copy link

dblock commented Apr 7, 2022

I found #84624 just now but that doesn’t fix the problem, it wraps the code to avoid the node crash

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/Aggregations Aggregations >bug :Search/Search Search-related issues that do not fall into other categories Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo)
Projects
None yet
5 participants