Skip to content

Commit e095a18

Browse files
mergify[bot]andsel
andauthored
Added test to verify the int overflow happen (#17353) (#17356)
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 afde43f) Co-authored-by: Andrea Selva <[email protected]>
1 parent 9228b1e commit e095a18

File tree

3 files changed

+34
-2
lines changed

3 files changed

+34
-2
lines changed

logstash-core/build.gradle

+2
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,8 @@ tasks.register("javaTests", Test) {
124124
exclude '/org/logstash/plugins/factory/PluginFactoryExtTest.class'
125125
exclude '/org/logstash/execution/ObservedExecutionTest.class'
126126

127+
maxHeapSize = "12g"
128+
127129
jacoco {
128130
enabled = true
129131
destinationFile = layout.buildDirectory.file('jacoco/test.exec').get().asFile

logstash-core/src/main/java/org/logstash/common/BufferedTokenizerExt.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public class BufferedTokenizerExt extends RubyObject {
4848
private RubyString delimiter = NEW_LINE;
4949
private int sizeLimit;
5050
private boolean hasSizeLimit;
51-
private int inputSize;
51+
private long inputSize;
5252
private boolean bufferFullErrorNotified = false;
5353
private String encodingName;
5454

logstash-core/src/test/java/org/logstash/common/BufferedTokenizerExtWithSizeLimitTest.java

+31-1
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,19 @@
3838
@SuppressWarnings("unchecked")
3939
public final class BufferedTokenizerExtWithSizeLimitTest extends RubyTestBase {
4040

41+
public static final int GB = 1024 * 1024 * 1024;
4142
private BufferedTokenizerExt sut;
4243
private ThreadContext context;
4344

4445
@Before
4546
public void setUp() {
47+
initSUTWithSizeLimit(10);
48+
}
49+
50+
private void initSUTWithSizeLimit(int sizeLimit) {
4651
sut = new BufferedTokenizerExt(RubyUtil.RUBY, RubyUtil.BUFFERED_TOKENIZER);
4752
context = RUBY.getCurrentContext();
48-
IRubyObject[] args = {RubyUtil.RUBY.newString("\n"), RubyUtil.RUBY.newFixnum(10)};
53+
IRubyObject[] args = {RubyUtil.RUBY.newString("\n"), RubyUtil.RUBY.newFixnum(sizeLimit)};
4954
sut.init(context, args);
5055
}
5156

@@ -108,4 +113,29 @@ public void giveMultipleSegmentsThatGeneratesMultipleBufferFullErrorsThenIsAbleT
108113
RubyArray<RubyString> tokens = (RubyArray<RubyString>) sut.extract(context, RubyUtil.RUBY.newString("ccc\nddd\n"));
109114
assertEquals(List.of("ccccc", "ddd"), tokens);
110115
}
116+
117+
@Test
118+
public void givenMaliciousInputExtractDoesntOverflow() {
119+
assertEquals("Xmx must equals to what's defined in the Gradle's javaTests task",
120+
12L * GB, Runtime.getRuntime().maxMemory());
121+
122+
// re-init the tokenizer with big sizeLimit
123+
initSUTWithSizeLimit((int) (2L * GB) - 3);
124+
// Integer.MAX_VALUE is 2 * GB
125+
String bigFirstPiece = generateString("a", Integer.MAX_VALUE - 1024);
126+
sut.extract(context, RubyUtil.RUBY.newString(bigFirstPiece));
127+
128+
// add another small fragment to trigger int overflow
129+
// sizeLimit is (2^32-1)-3 first segment length is (2^32-1) - 1024 second is 1024 +2
130+
// so the combined length of first and second is > sizeLimit and should throw an expection
131+
// but because of overflow it's negative and happens to be < sizeLimit
132+
Exception thrownException = assertThrows(IllegalStateException.class, () -> {
133+
sut.extract(context, RubyUtil.RUBY.newString(generateString("a", 1024 + 2)));
134+
});
135+
assertThat(thrownException.getMessage(), containsString("input buffer full"));
136+
}
137+
138+
private String generateString(String fill, int size) {
139+
return fill.repeat(size);
140+
}
111141
}

0 commit comments

Comments
 (0)