fix: validate RLBE decoder run lengths - #913
Conversation
b30daa6 to
b8a2a83
Compare
There was a problem hiding this comment.
Pull request overview
This PR hardens RLBE decoding in both the Java and C++ implementations to better defend against malformed/corrupted encoded data (preventing out-of-bounds access, overflow, and invalid decoded output) and adds regression coverage for key failure cases.
Changes:
- Java: add RLBE block size validation and stricter Fibonacci/run-length validation with
TsFileDecodingException. - C++: tighten RLBE block size validation and add Fibonacci bounds/overflow + remaining-length checks returning
E_TSFILE_CORRUPTED. - Tests: add Java and C++ regression tests for zero block size and run length exceeding the declared block size.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| java/tsfile/src/test/java/org/apache/tsfile/encoding/decoder/RLBEDecoderTest.java | Adds Java regression tests for invalid RLBE block size and excessive run length. |
| java/tsfile/src/main/java/org/apache/tsfile/encoding/decoder/LongRLBEDecoder.java | Adds block size validation and stronger Fibonacci/run-length validation for long RLBE decoding. |
| java/tsfile/src/main/java/org/apache/tsfile/encoding/decoder/IntRLBEDecoder.java | Adds block size validation and stronger Fibonacci/run-length validation for int RLBE decoding. |
| cpp/test/encoding/rlbe_codec_test.cc | Adds C++ regression tests for invalid RLBE block size and excessive run length. |
| cpp/src/encoding/rlbe_decoder.h | Tightens C++ RLBE decoding validation (block size, Fibonacci bounds/overflow, remaining-length checks). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| int j = 1; | ||
| while (true) { | ||
| if (j >= fibonacci.length) { | ||
| throw new TsFileDecodingException("Invalid RLBE Fibonacci run length"); | ||
| } |
| int j = 1; | ||
| while (true) { | ||
| if (j >= fibonacci.length) { | ||
| throw new TsFileDecodingException("Invalid RLBE Fibonacci run length"); | ||
| } |
| int j = 1; | ||
| while (true) { | ||
| if (j >= static_cast<int>(sizeof(fibonacci_) / | ||
| sizeof(fibonacci_[0]))) { | ||
| return common::E_TSFILE_CORRUPTED; |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Suppressed comments (4)
Previously missed (2) — in code that hasn't changed since the last review.
java/tsfile/src/main/java/org/apache/tsfile/encoding/decoder/IntRLBEDecoder.java:105
- The decoder still reads bits via
readbit()/loadBuffer()which callbuffer.get()without checkingbuffer.remaining(). For truncated/corrupted RLBE input, this will throwBufferUnderflowExceptionrather than the intendedTsFileDecodingException, and contradicts the goal of rejecting malformed input with a decoding error. Consider guardingloadBuffer(orreadbit) and throwingTsFileDecodingExceptionon unexpected EOF.
This issue also appears on line 107 of the same file.
for (int j = 5; j >= 0; j--) {
seglength |= (readbit(buffer) << j);
}
if (seglength < 1 || seglength > 32) {
throw new TsFileDecodingException("Invalid RLBE segment length: " + seglength);
}
// generate repeat time of rle on delta
int now = readbit(buffer);
int next = readbit(buffer);
java/tsfile/src/main/java/org/apache/tsfile/encoding/decoder/LongRLBEDecoder.java:106
- Like
IntRLBEDecoder, this decoder ultimately reads viabuffer.get()insideloadBuffer()without checkingbuffer.remaining(). Truncated/corrupted RLBE input can therefore throwBufferUnderflowExceptioninstead of the expectedTsFileDecodingException. To make malformed input consistently report a decoding error, guardloadBuffer/readbitand throwTsFileDecodingExceptionon unexpected EOF.
This issue also appears on line 108 of the same file.
for (int j = 6; j >= 0; j--) {
seglength |= (readbit(buffer) << j);
}
if (seglength < 1 || seglength > 64) {
throw new TsFileDecodingException("Invalid RLBE segment length: " + seglength);
}
// generate repeat time of rle on delta
int now = readbit(buffer);
int next = readbit(buffer);
java/tsfile/src/main/java/org/apache/tsfile/encoding/decoder/IntRLBEDecoder.java:118
fibonacci[j] = fibonacci[j - 1] + fibonacci[j - 2]can overflowintand wrap back to a small positive value, which can then pass the currentcandidatebounds check and produce an invalidrunlengthinstead of rejecting corrupted data. To reliably detect overflow, add an explicit monotonic/overflow guard after computingfibonacci[j](e.g., reject iffibonacci[j] <= 0orfibonacci[j] <= fibonacci[j - 1]).
while (true) {
if (j >= fibonacci.length) {
throw new TsFileDecodingException("Invalid RLBE Fibonacci run length");
}
if (j > 1) fibonacci[j] = fibonacci[j - 1] + fibonacci[j - 2];
if (now == 1) {
long candidate = (long) runlength + fibonacci[j];
if (candidate <= 0 || candidate > blocksize - writeindex - 1) {
throw new TsFileDecodingException("Invalid RLBE run length: " + candidate);
}
runlength = (int) candidate;
}
java/tsfile/src/main/java/org/apache/tsfile/encoding/decoder/LongRLBEDecoder.java:119
fibonacci[j] = fibonacci[j - 1] + fibonacci[j - 2]can overflowlongfor sufficiently long malformed Fibonacci codes; after overflow it may wrap back to a positive value and bypass the currentvalue <= 0check, allowing an invalidrunlengthto be accepted. Add an explicit overflow/monotonicity check after computingfibonacci[j](e.g., reject iffibonacci[j] <= fibonacci[j - 1]).
while (true) {
if (j >= fibonacci.length) {
throw new TsFileDecodingException("Invalid RLBE Fibonacci run length");
}
if (j > 1) fibonacci[j] = fibonacci[j - 1] + fibonacci[j - 2];
if (now == 1) {
long value = fibonacci[j];
if (value <= 0 || runlength > blocksize - writeindex - 1 - value) {
throw new TsFileDecodingException("Invalid RLBE run length");
}
runlength += value;
}
| return ret; | ||
| } | ||
| block_size_ = static_cast<int>(bits); | ||
| if (block_size_ < 0 || block_size_ > RLBE_BLOCK_DEFAULT_SIZE) { | ||
| return common::E_TSFILE_CORRUPTED; | ||
| if (block_size_ <= 0 || block_size_ > RLBE_BLOCK_DEFAULT_SIZE) { | ||
| return common::E_DECODE_ERR; | ||
| } |
Summary
Fix RLBE decoder validation in both Java and C++ to prevent malformed input from causing out-of-bounds access, integer overflow, excessive reads, or invalid decoded output.
Changes
TsFileDecodingExceptionE_TSFILE_CORRUPTEDMotivation
Previously, the RLBE decoders trusted the encoded run length without checking it against the declared block size. A malformed or corrupted TsFile could therefore cause array index errors, buffer underflow, integer overflow, or the decoding of more values than declared.
Testing
RLBEDecoderTest: 10 tests passed.rlbe_codec_test.cc.git diff --checkpassed.