Skip to content

Commit

Permalink
Avoid overflow in index input slices invariant checks (#14126)
Browse files Browse the repository at this point in the history
This commit avoids overflow in index input slices invariant checks.

While not a problem in practice, this can lead to more obscure failures which are harder to diagnose. Reworking the invariant checks to avoid the potential to overflow is trivial. Existing tests cover the most cases, while a single new scenario covered the overflow case.
  • Loading branch information
ChrisHegarty authored Jan 20, 2025
1 parent dce24c8 commit 7e20d5b
Show file tree
Hide file tree
Showing 8 changed files with 15 additions and 6 deletions.
3 changes: 3 additions & 0 deletions lucene/CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ Bug Fixes

* GITHUB#14123: SortingCodecReader NPE when segment has no (points, vectors, etc...) (Mike Sokolov)

* GITHUB#14126: Avoid overflow in index input slices invariant checks
(Chris Hegarty)

Other
---------------------

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ private static final class SlicedIndexInput extends BufferedIndexInput {
? base.toString()
: (base.toString() + " [slice=" + sliceDescription + "]"),
BufferedIndexInput.BUFFER_SIZE);
if (offset < 0 || length < 0 || offset + length > base.length()) {
if ((length | offset) < 0 || length > base.length() - offset) {
throw new IllegalArgumentException(
"slice() " + sliceDescription + " out of bounds: " + base);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ public void skipBytes(long numBytes) throws IOException {
}

public ByteBuffersDataInput slice(long offset, long length) {
if (offset < 0 || length < 0 || offset + length > this.length) {
if ((length | offset) < 0 || length > this.length - offset) {
throw new IllegalArgumentException(
String.format(
Locale.ROOT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ public NIOFSIndexInput clone() {

@Override
public IndexInput slice(String sliceDescription, long offset, long length) throws IOException {
if (offset < 0 || length < 0 || offset + length > this.length()) {
if ((length | offset) < 0 || length > this.length() - offset) {
throw new IllegalArgumentException(
"slice() "
+ sliceDescription
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,7 @@ public final MemorySegmentIndexInput clone() {
*/
@Override
public final MemorySegmentIndexInput slice(String sliceDescription, long offset, long length) {
if (offset < 0 || length < 0 || offset + length > this.length) {
if ((length | offset) < 0 || length > this.length - offset) {
throw new IllegalArgumentException(
"slice() "
+ sliceDescription
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ public RAFIndexInput clone() {

@Override
public IndexInput slice(String sliceDescription, long offset, long length) throws IOException {
if (offset < 0 || length < 0 || offset + length > this.length()) {
if ((length | offset) < 0 || length > this.length() - offset) {
throw new IllegalArgumentException(
"slice() " + sliceDescription + " out of bounds: " + this);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -771,6 +771,12 @@ public void testSliceOutOfBounds() throws Exception {
slice.slice("slice3sub", 1, len / 2);
});

expectThrows(
IllegalArgumentException.class,
() -> {
i.slice("slice4", Long.MAX_VALUE - 1, 10);
});

i.close();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ public IndexInput slice(String sliceDescription, long offset, long length) throw
public IndexInput slice(
String sliceDescription, long offset, long length, ReadAdvice readAdvice)
throws IOException {
if (offset < 0 || offset + length > sliceLength) {
if ((length | offset) < 0 || length > sliceLength - offset) {
throw new IllegalArgumentException();
}
IndexInput clone = in.clone();
Expand Down

0 comments on commit 7e20d5b

Please sign in to comment.