From ef16be930388fd527244843cd21ca8251cd5f67a Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Thu, 11 Apr 2024 15:59:53 -0500 Subject: [PATCH] Catching StackOverflowErrors from bad regexes in GsubProcessor and rethrowing as an Exception (#106851) --- docs/changelog/106851.yaml | 5 ++++ .../ingest/common/GsubProcessor.java | 18 ++++++++++++++- .../ingest/common/GsubProcessorTests.java | 23 +++++++++++++++++++ 3 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 docs/changelog/106851.yaml diff --git a/docs/changelog/106851.yaml b/docs/changelog/106851.yaml new file mode 100644 index 0000000000000..2ada6a6a4e088 --- /dev/null +++ b/docs/changelog/106851.yaml @@ -0,0 +1,5 @@ +pr: 106851 +summary: Catching `StackOverflowErrors` from bad regexes in `GsubProcessor` +area: Ingest Node +type: bug +issues: [] diff --git a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/GsubProcessor.java b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/GsubProcessor.java index d93ca025469dd..ea93a589f3c97 100644 --- a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/GsubProcessor.java +++ b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/GsubProcessor.java @@ -8,6 +8,9 @@ package org.elasticsearch.ingest.common; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + import java.util.Map; import java.util.regex.Pattern; @@ -21,6 +24,7 @@ public final class GsubProcessor extends AbstractStringProcessor { public static final String TYPE = "gsub"; + private static final Logger logger = LogManager.getLogger(GsubProcessor.class); private final Pattern pattern; private final String replacement; @@ -49,7 +53,19 @@ String getReplacement() { @Override protected String process(String value) { - return pattern.matcher(value).replaceAll(replacement); + try { + return pattern.matcher(value).replaceAll(replacement); + } catch (StackOverflowError e) { + /* + * A bad regex on problematic data can trigger a StackOverflowError. In this case we can safely recover from the + * StackOverflowError, so we rethrow it as an Exception instead. This way the document fails this processor, but processing + * can carry on. The value would be useful to log here, but we do not do so for because we do not want to write potentially + * sensitive data to the logs. + */ + String message = "Caught a StackOverflowError while processing gsub pattern: [" + pattern + "]"; + logger.trace(message, e); + throw new IllegalArgumentException(message); + } } @Override diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/GsubProcessorTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/GsubProcessorTests.java index e5fe45d0c381f..6936ad11a785a 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/GsubProcessorTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/GsubProcessorTests.java @@ -8,6 +8,10 @@ package org.elasticsearch.ingest.common; +import org.elasticsearch.ingest.IngestDocument; +import org.elasticsearch.ingest.RandomDocumentPicks; + +import java.util.Map; import java.util.regex.Pattern; public class GsubProcessorTests extends AbstractStringProcessorTestCase { @@ -26,4 +30,23 @@ protected String modifyInput(String input) { protected String expectedResult(String input) { return "127-0-0-1"; } + + public void testStackOverflow() { + // This tests that we rethrow StackOverflowErrors as ElasticsearchExceptions so that we don't take down the node + String badRegex = "( (?=(?:[^'\"]|'[^']*'|\"[^\"]*\")*$))"; + GsubProcessor processor = new GsubProcessor( + randomAlphaOfLength(10), + null, + "field", + Pattern.compile(badRegex), + "-", + false, + "targetField" + ); + StringBuilder badSourceBuilder = new StringBuilder("key1=x key2="); + badSourceBuilder.append("x".repeat(3000)); + Map source = Map.of("field", badSourceBuilder.toString()); + IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), source); + IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> processor.execute(ingestDocument)); + } }