From a403b11dcae0956f19ab65ebceb0e529f1a0770d Mon Sep 17 00:00:00 2001 From: Andrea Selva Date: Wed, 19 Mar 2025 16:50:04 +0100 Subject: [PATCH] Added test to verify the int overflow happen (#17353) Use long instead of int type to keep the length of the first token. The size limit validation requires to sum two integers, one with the length of the accumulated chars till now plus the next fragment head part. If any of the two sizes is close to the max integer it generates an overflow and could successfully fail the test https://github.com/elastic/logstash/blob/9c0e50faacc4700da3dc84a3ba729b84bff860a8/logstash-core/src/main/java/org/logstash/common/BufferedTokenizerExt.java#L123. To fall in this case it's required that sizeLimit is bigger then 2^32 bytes (2GB) and data fragments without any line delimiter is pushed to the tokenizer with a total size close to 2^32 bytes. (cherry picked from commit afde43f9188f9a782b6d78476fd82840d703f7c2) --- logstash-core/build.gradle | 2 ++ .../logstash/common/BufferedTokenizerExt.java | 2 +- ...BufferedTokenizerExtWithSizeLimitTest.java | 32 ++++++++++++++++++- 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/logstash-core/build.gradle b/logstash-core/build.gradle index 30c64040c33..2b3bb6e6688 100644 --- a/logstash-core/build.gradle +++ b/logstash-core/build.gradle @@ -124,6 +124,8 @@ tasks.register("javaTests", Test) { exclude '/org/logstash/plugins/factory/PluginFactoryExtTest.class' exclude '/org/logstash/execution/ObservedExecutionTest.class' + maxHeapSize = "12g" + jacoco { enabled = true destinationFile = layout.buildDirectory.file('jacoco/test.exec').get().asFile diff --git a/logstash-core/src/main/java/org/logstash/common/BufferedTokenizerExt.java b/logstash-core/src/main/java/org/logstash/common/BufferedTokenizerExt.java index e2c476520c1..2161dc90049 100644 --- a/logstash-core/src/main/java/org/logstash/common/BufferedTokenizerExt.java +++ b/logstash-core/src/main/java/org/logstash/common/BufferedTokenizerExt.java @@ -48,7 +48,7 @@ public class BufferedTokenizerExt extends RubyObject { private RubyString delimiter = NEW_LINE; private int sizeLimit; private boolean hasSizeLimit; - private int inputSize; + private long inputSize; private boolean bufferFullErrorNotified = false; private String encodingName; diff --git a/logstash-core/src/test/java/org/logstash/common/BufferedTokenizerExtWithSizeLimitTest.java b/logstash-core/src/test/java/org/logstash/common/BufferedTokenizerExtWithSizeLimitTest.java index 9a07242369d..16181bb6f18 100644 --- a/logstash-core/src/test/java/org/logstash/common/BufferedTokenizerExtWithSizeLimitTest.java +++ b/logstash-core/src/test/java/org/logstash/common/BufferedTokenizerExtWithSizeLimitTest.java @@ -38,14 +38,19 @@ @SuppressWarnings("unchecked") public final class BufferedTokenizerExtWithSizeLimitTest extends RubyTestBase { + public static final int GB = 1024 * 1024 * 1024; private BufferedTokenizerExt sut; private ThreadContext context; @Before public void setUp() { + initSUTWithSizeLimit(10); + } + + private void initSUTWithSizeLimit(int sizeLimit) { sut = new BufferedTokenizerExt(RubyUtil.RUBY, RubyUtil.BUFFERED_TOKENIZER); context = RUBY.getCurrentContext(); - IRubyObject[] args = {RubyUtil.RUBY.newString("\n"), RubyUtil.RUBY.newFixnum(10)}; + IRubyObject[] args = {RubyUtil.RUBY.newString("\n"), RubyUtil.RUBY.newFixnum(sizeLimit)}; sut.init(context, args); } @@ -108,4 +113,29 @@ public void giveMultipleSegmentsThatGeneratesMultipleBufferFullErrorsThenIsAbleT RubyArray tokens = (RubyArray) sut.extract(context, RubyUtil.RUBY.newString("ccc\nddd\n")); assertEquals(List.of("ccccc", "ddd"), tokens); } + + @Test + public void givenMaliciousInputExtractDoesntOverflow() { + assertEquals("Xmx must equals to what's defined in the Gradle's javaTests task", + 12L * GB, Runtime.getRuntime().maxMemory()); + + // re-init the tokenizer with big sizeLimit + initSUTWithSizeLimit((int) (2L * GB) - 3); + // Integer.MAX_VALUE is 2 * GB + String bigFirstPiece = generateString("a", Integer.MAX_VALUE - 1024); + sut.extract(context, RubyUtil.RUBY.newString(bigFirstPiece)); + + // add another small fragment to trigger int overflow + // sizeLimit is (2^32-1)-3 first segment length is (2^32-1) - 1024 second is 1024 +2 + // so the combined length of first and second is > sizeLimit and should throw an expection + // but because of overflow it's negative and happens to be < sizeLimit + Exception thrownException = assertThrows(IllegalStateException.class, () -> { + sut.extract(context, RubyUtil.RUBY.newString(generateString("a", 1024 + 2))); + }); + assertThat(thrownException.getMessage(), containsString("input buffer full")); + } + + private String generateString(String fill, int size) { + return fill.repeat(size); + } } \ No newline at end of file