From 5305a784580e1330eb56aac46c0cb10d13c0da17 Mon Sep 17 00:00:00 2001
From: Tyler Gregg <greggt@amazon.com>
Date: Thu, 25 Jan 2024 14:46:17 -0800
Subject: [PATCH] Fixes a bug that allowed NegativeArraySizeException to be
 thrown from IonReader.newBytes().

---
 .../com/amazon/ion/impl/IonCursorBinary.java  | 25 +++++++++++--------
 .../IonReaderContinuableCoreBinaryTest.java   | 16 ++++++++++++
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/src/main/java/com/amazon/ion/impl/IonCursorBinary.java b/src/main/java/com/amazon/ion/impl/IonCursorBinary.java
index 9cb9c91225..1bfe194c28 100644
--- a/src/main/java/com/amazon/ion/impl/IonCursorBinary.java
+++ b/src/main/java/com/amazon/ion/impl/IonCursorBinary.java
@@ -898,17 +898,16 @@ private long calculateEndIndex_1_0(IonTypeID valueTid, boolean isAnnotated) {
             }
             byte b = buffer[(int) peekIndex++];
             if (b < 0) {
-                endIndex = (b & LOWER_SEVEN_BITS_BITMASK);
+                endIndex = (b & LOWER_SEVEN_BITS_BITMASK) + peekIndex;
             } else {
-                endIndex = uncheckedReadVarUInt_1_0(b);
+                endIndex = uncheckedReadVarUInt_1_0(b) + peekIndex;
                 if (endIndex < 0) {
                     throw new IonException("Unsupported value: declared length is too long.");
                 }
             }
         } else {
-            endIndex = valueTid.length;
+            endIndex = valueTid.length + peekIndex;
         }
-        endIndex += peekIndex;
         if (valueTid.type != null && valueTid.type.ordinal() >= LIST_TYPE_ORDINAL) {
             event = Event.START_CONTAINER;
         } else if (valueTid.isNopPad) {
@@ -1308,13 +1307,19 @@ private boolean slowReadValueHeader(IonTypeID valueTid, boolean isAnnotated, Mar
             setCheckpoint(CheckpointLocation.AFTER_SCALAR_HEADER);
             event = Event.START_SCALAR;
         }
-        if (refillableState.isSkippingCurrentValue) {
-            // Any bytes that were skipped directly from the input must still be included in the logical endIndex so
-            // that the rest of the oversized value's bytes may be skipped.
-            endIndex = endIndex == DELIMITED_MARKER ? DELIMITED_MARKER : (peekIndex + valueLength + refillableState.individualBytesSkippedWithoutBuffering);
-        } else {
-            endIndex = endIndex == DELIMITED_MARKER ? DELIMITED_MARKER : (peekIndex + valueLength);
+        if (endIndex != DELIMITED_MARKER) {
+            if (refillableState.isSkippingCurrentValue) {
+                // Any bytes that were skipped directly from the input must still be included in the logical endIndex so
+                // that the rest of the oversized value's bytes may be skipped.
+                endIndex = peekIndex + valueLength + refillableState.individualBytesSkippedWithoutBuffering;
+            } else {
+                endIndex = peekIndex + valueLength;
+            }
+            if (endIndex < 0) {
+                throw new IonException("Unsupported value: declared length is too long.");
+            }
         }
+
         if (isAnnotated) {
             validateAnnotationWrapperEndIndex(endIndex);
         }
diff --git a/src/test/java/com/amazon/ion/impl/IonReaderContinuableCoreBinaryTest.java b/src/test/java/com/amazon/ion/impl/IonReaderContinuableCoreBinaryTest.java
index 874747ef31..0fc2338b5f 100644
--- a/src/test/java/com/amazon/ion/impl/IonReaderContinuableCoreBinaryTest.java
+++ b/src/test/java/com/amazon/ion/impl/IonReaderContinuableCoreBinaryTest.java
@@ -480,4 +480,20 @@ public void expectAttemptToParseNullDecimalAsJavaPrimitiveToFailCleanly() {
         assertReaderThrowsForAllPrimitives(reader);
         reader.close();
     }
+
+    @ParameterizedTest(name = "constructFromBytes={0}")
+    @ValueSource(booleans = {true, false})
+    public void expectLobWithOverflowingEndIndexToFailCleanly(boolean constructFromBytes) {
+        // This test exercises the case where a value's length itself does not overflow a java long, but its end index
+        // (calculated by adding the reader's current index in the buffer to the value's length) does.
+        IonReaderContinuableCoreBinary reader = initializeReader(
+            constructFromBytes,
+            0xE0, 0x01, 0x00, 0xEA, // IVM
+            0xAE, // blob with length VarUInt to follow
+            0x7F, 0x7F, 0x7F, 0x7F, 0x7F, 0x7F, 0x7F, 0x7F, 0xFE, // 9-byte VarUInt with value just below Long.MAX_VALUE
+            0xAF // The first byte of the blob
+        );
+        assertThrows(IonException.class, reader::nextValue);
+        reader.close();
+    }
 }