Fix integer overflow in bounds checking

This commit is contained in:
Martin Dindoffer 2022-04-28 14:59:56 +02:00 committed by Vaci
parent ced3f11e6f
commit e77372b1cd
4 changed files with 54 additions and 6 deletions

View file

@ -39,11 +39,13 @@ public class SegmentReader {
return buffer.getLong(index * Constants.BYTES_PER_WORD); return buffer.getLong(index * Constants.BYTES_PER_WORD);
} }
// Verify that the `size`-long (in words) range starting at word index /**
// `start` is within bounds. * Verify that the `size`-long (in words) range starting at word index
public final boolean in_bounds(int start, int size) { * `start` is within bounds.
*/
public final boolean isInBounds(int start, int size) {
if (start < 0 || size < 0) return false; if (start < 0 || size < 0) return false;
long sizeInWords = size * Constants.BYTES_PER_WORD; long sizeInWords = (long) size * Constants.BYTES_PER_WORD;
return (long) start + sizeInWords <= (long) this.buffer.capacity(); return start + sizeInWords <= this.buffer.capacity();
} }
} }

View file

@ -43,7 +43,7 @@ final class WireHelpers {
static boolean bounds_check(SegmentReader segment, static boolean bounds_check(SegmentReader segment,
int start, int start,
int size) { int size) {
return segment == null || segment.in_bounds(start, size); return segment == null || segment.isInBounds(start, size);
} }
static class AllocateResult { static class AllocateResult {

View file

@ -86,6 +86,34 @@ public class LayoutTest {
StructReader reader = WireHelpers.readStructPointer(new BareStructReader(), arena.tryGetSegment(0), null, 0, null, 0, 0x7fffffff); StructReader reader = WireHelpers.readStructPointer(new BareStructReader(), arena.tryGetSegment(0), null, 0, null, 0, 0x7fffffff);
} }
private static class BareListReader implements ListReader.Factory<ListReader> {
BareListReader() {
}
@Override
public ListReader constructReader(SegmentReader segment, int ptr, int elementCount, int step, int structDataSize, short structPointerCount, int nestingLimit) {
return new ListReader(segment, ptr, elementCount, step, structDataSize, structPointerCount, nestingLimit);
}
}
@Test(expected = DecodeException.class)
public void readListPointerShouldThrowDecodeExceptionOnOutOfBoundsCompositeListPointer() {
byte[] brokenMSG = {
// set list pointer bits to 1, elementSize to 7 to indicate composite list and number of words in the list (minus tag) to 0x1FFFFFFF (max value possible in 29b limit)
0x01, 0x00, 0x00, 0x00, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF,
0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,//tag with element wordSize of 1
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
};
ByteBuffer buffer = ByteBuffer.wrap(brokenMSG);
buffer.order(ByteOrder.LITTLE_ENDIAN);
ReaderArena arena = new ReaderArena(new ByteBuffer[]{buffer}, 0x7fffffffffffffffL);
ListReader reader = WireHelpers.readListPointer(new BareListReader(), arena.tryGetSegment(0), 0, null, 0, (byte) 0, 0x7fffffff);
}
private class BareStructBuilder implements StructBuilder.Factory<StructBuilder> { private class BareStructBuilder implements StructBuilder.Factory<StructBuilder> {
private StructSize structSize; private StructSize structSize;

View file

@ -0,0 +1,18 @@
package org.capnproto;
import org.hamcrest.MatcherAssert;
import org.junit.Test;
import java.nio.ByteBuffer;
import static org.hamcrest.CoreMatchers.is;
public class SegmentReaderTest {
@Test
public void in_boundsCalculationShouldNotOverflow() {
ByteBuffer byteBuffer = ByteBuffer.allocate(64);
SegmentReader segmentReader = new SegmentReader(byteBuffer, null);
MatcherAssert.assertThat(segmentReader.isInBounds(0, Integer.MAX_VALUE), is(false));
}
}