The bytes parameter of the copy method is not carefully checked. [LUCENE-8536]
- Dominant language
- Java
- Stars
- 3.6k
- Forks
- 1.4k
- Avg merge
- 2d 11h
- Merged PRs (30d)
- 88
Description
The copy method of the PagedBytes class is as follow:
```java
public void copy(BytesRef bytes, BytesRef out) {
int left = blockSize - upto;
if (bytes.length > left || currentBlock==null) {
if (currentBlock != null) {
addBlock(currentBlock);
didSkipBytes = true;
}
currentBlock = new byte[blockSize];
upto = 0;
left = blockSize;
assert bytes.length <= blockSize;
// TODO: we could also support variable block sizes
}
out.bytes = currentBlock;
out.offset = upto;
out.length = bytes.length;
System.arraycopy(bytes.bytes, bytes.offset, currentBlock, upto, bytes.length);
upto += bytes.length;
}
```
The method does not throw exceptions for illegal inputs. In the same class, the copyUsingLengthPrefix method checks the input value"
```java
public long copyUsingLengthPrefix(BytesRef bytes) {
if (bytes.length >= 32768) {
throw new IllegalArgumentException("max length is 32767 (got " + bytes.length + ")");
}
if (upto + bytes.length + 2 > blockSize) {
if (bytes.length + 2 > blockSize) {
throw new IllegalArgumentException("block size " + blockSize + " is too small to store length " + bytes.length + " bytes");
}
if (currentBlock != null) {
addBlock(currentBlock);
}
currentBlock = new byte[blockSize];
upto = 0;
}
final long pointer = getPointer();
if (bytes.length < 128) {
currentBlock[upto++] = (byte) bytes.length;
} else {
currentBlock[upto++] = (byte) (0x80 | (bytes.length >> 8));
currentBlock[upto++] = (byte) (bytes.length & 0xff);
}
System.arraycopy(bytes.bytes, bytes.offset, currentBlock, upto, bytes.length);
upto += bytes.length;
return pointer;
}
```
I understand that in the first method,
```java
assert bytes.length <= blockSize;
```
checks whether the length of the bytes is too large. However, the method does not check blockSize either. As a result, the length of the bytes can still be overflowed, if blockSize is too large. In addition, the second method also checks whether
```java
bytes.length + 2 > blockSize
```
Shall the first method also checks the requirement?
---
Migrated from [LUCENE-8536](https://issues.apache.org/jira/browse/LUCENE-8536) by Hao Zhong, updated Apr 14 2019
Contributor guide
Research direction
Start at the PagedBytes.copy method and compare its input handling with copyUsingLengthPrefix; inspect callers to understand the blockSize assumptions. The payload names no file or test, so locate relevant coverage and confirm the intended behavior for inputs that exceed the block size, including that illegal lengths are rejected without overflow.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100