From e77372b1cdc27786490f7872d43bc6ca6a2c1669 Mon Sep 17 00:00:00 2001
From: Martin Dindoffer <contact@dindoffer.eu>
Date: Thu, 28 Apr 2022 14:59:56 +0200
Subject: [PATCH] Fix integer overflow in bounds checking

---
 .../java/org/capnproto/SegmentReader.java     | 12 ++++----
 .../main/java/org/capnproto/WireHelpers.java  |  2 +-
 .../test/java/org/capnproto/LayoutTest.java   | 28 +++++++++++++++++++
 .../java/org/capnproto/SegmentReaderTest.java | 18 ++++++++++++
 4 files changed, 54 insertions(+), 6 deletions(-)
 create mode 100644 runtime/src/test/java/org/capnproto/SegmentReaderTest.java

diff --git a/runtime/src/main/java/org/capnproto/SegmentReader.java b/runtime/src/main/java/org/capnproto/SegmentReader.java
index 62a30ef..d24ba34 100644
--- a/runtime/src/main/java/org/capnproto/SegmentReader.java
+++ b/runtime/src/main/java/org/capnproto/SegmentReader.java
@@ -39,11 +39,13 @@ public class SegmentReader {
         return buffer.getLong(index * Constants.BYTES_PER_WORD);
     }
 
-    // Verify that the `size`-long (in words) range starting at word index
-    // `start` is within bounds.
-    public final boolean in_bounds(int start, int size) {
+    /**
+     * Verify that the `size`-long (in words) range starting at word index
+     * `start` is within bounds.
+     */
+    public final boolean isInBounds(int start, int size) {
         if (start < 0 || size < 0) return false;
-        long sizeInWords = size * Constants.BYTES_PER_WORD;
-        return (long) start + sizeInWords <= (long) this.buffer.capacity();
+        long sizeInWords = (long) size * Constants.BYTES_PER_WORD;
+        return start + sizeInWords <= this.buffer.capacity();
     }
 }
diff --git a/runtime/src/main/java/org/capnproto/WireHelpers.java b/runtime/src/main/java/org/capnproto/WireHelpers.java
index 6e09668..8d85160 100644
--- a/runtime/src/main/java/org/capnproto/WireHelpers.java
+++ b/runtime/src/main/java/org/capnproto/WireHelpers.java
@@ -43,7 +43,7 @@ final class WireHelpers {
     static boolean bounds_check(SegmentReader segment,
                                 int start,
                                 int size) {
-        return segment == null || segment.in_bounds(start, size);
+        return segment == null || segment.isInBounds(start, size);
     }
 
     static class AllocateResult {
diff --git a/runtime/src/test/java/org/capnproto/LayoutTest.java b/runtime/src/test/java/org/capnproto/LayoutTest.java
index b885fcd..a4dd831 100644
--- a/runtime/src/test/java/org/capnproto/LayoutTest.java
+++ b/runtime/src/test/java/org/capnproto/LayoutTest.java
@@ -86,6 +86,34 @@ public class LayoutTest {
         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 StructSize structSize;
 
diff --git a/runtime/src/test/java/org/capnproto/SegmentReaderTest.java b/runtime/src/test/java/org/capnproto/SegmentReaderTest.java
new file mode 100644
index 0000000..4678963
--- /dev/null
+++ b/runtime/src/test/java/org/capnproto/SegmentReaderTest.java
@@ -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));
+    }
+}
\ No newline at end of file