From 1d72272e14e47cdff854467c87ccf762cd1bccac Mon Sep 17 00:00:00 2001 From: Tyler Gregg Date: Tue, 2 Jul 2024 17:16:55 -0700 Subject: [PATCH] Fixes handling of annotation wrapper length overflows. --- .../com/amazon/ion/impl/IonCursorBinary.java | 12 +++-- .../amazon/ion/impl/IonCursorBinaryTest.java | 50 +++++++++++++++++++ 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/amazon/ion/impl/IonCursorBinary.java b/src/main/java/com/amazon/ion/impl/IonCursorBinary.java index 6166cfc38..0014535e7 100644 --- a/src/main/java/com/amazon/ion/impl/IonCursorBinary.java +++ b/src/main/java/com/amazon/ion/impl/IonCursorBinary.java @@ -502,6 +502,9 @@ private long availableAt(long index) { * @return true if the buffer has sufficient capacity; otherwise, false. */ private boolean ensureCapacity(long minimumNumberOfBytesRequired) { + if (minimumNumberOfBytesRequired < 0) { + throw new IonException("The number of bytes required cannot be represented in a Java long."); + } if (freeSpaceAt(offset) >= minimumNumberOfBytesRequired) { // No need to shift any bytes or grow the buffer. return true; @@ -809,11 +812,11 @@ private boolean uncheckedReadAnnotationWrapperHeader_1_0(IonTypeID valueTid) { throw new IonException("Malformed data: declared length exceeds the number of bytes remaining in the stream."); } byte b = buffer[(int) peekIndex++]; - int annotationsLength; + long annotationsLength; if (b < 0) { annotationsLength = (b & LOWER_SEVEN_BITS_BITMASK); } else { - annotationsLength = (int) uncheckedReadVarUInt_1_0(b); + annotationsLength = uncheckedReadVarUInt_1_0(b); } annotationSequenceMarker.startIndex = peekIndex; annotationSequenceMarker.endIndex = annotationSequenceMarker.startIndex + annotationsLength; @@ -821,6 +824,9 @@ private boolean uncheckedReadAnnotationWrapperHeader_1_0(IonTypeID valueTid) { if (peekIndex >= endIndex) { throw new IonException("Annotation wrapper must wrap a value."); } + if (peekIndex < 0) { + throw new IonException("Malformed data: declared length exceeds the number of bytes remaining in the stream."); + } return false; } @@ -855,7 +861,7 @@ private boolean slowReadAnnotationWrapperHeader_1_0(IonTypeID valueTid) { } // Record the post-length index in a value that will be shifted in the event the buffer needs to refill. setMarker(peekIndex + valueLength, valueMarker); - int annotationsLength = (int) slowReadVarUInt_1_0(); + long annotationsLength = slowReadVarUInt_1_0(); if (annotationsLength < 0) { return true; } diff --git a/src/test/java/com/amazon/ion/impl/IonCursorBinaryTest.java b/src/test/java/com/amazon/ion/impl/IonCursorBinaryTest.java index 38336067b..fb0e99e57 100644 --- a/src/test/java/com/amazon/ion/impl/IonCursorBinaryTest.java +++ b/src/test/java/com/amazon/ion/impl/IonCursorBinaryTest.java @@ -467,4 +467,54 @@ public void expectIncompleteIvmToFailCleanly(boolean constructFromBytes) { } cursor.close(); } + + @ParameterizedTest(name = "constructFromBytes={0}") + @ValueSource(booleans = {true, false}) + public void annotationSequenceLengthExceedsJavaLong(boolean constructFromBytes) { + try ( + IonCursorBinary cursor = initializeCursor( + constructFromBytes, + 0xE0, 0x01, 0x00, 0xEA, // IVM + 0xEA, // Annotation wrapper, length 10 + 0x01, 0x16, 0x76, 0x76, 0x76, 0x76, 0x3F, 0x3F, 0x76, 0x76, 0xF3, // Annotation sequence length > Long.MAX_VALUE + 0x9E // The start of the annotation sequence + ) + ) { + assertThrows(IonException.class, cursor::nextValue); + } + } + + @ParameterizedTest(name = "constructFromBytes={0}") + @ValueSource(booleans = {true, false}) + public void annotationSequenceLengthPlusCurrentIndexExceedsJavaLong(boolean constructFromBytes) { + try ( + IonCursorBinary cursor = initializeCursor( + constructFromBytes, + 0xE0, 0x01, 0x00, 0xEA, // IVM + 0xEA, // Annotation wrapper, length 10 + 0x7F, 0x7F, 0x7F, 0x7F, 0x7F, 0x7F, 0x7F, 0x7F, 0xFF, // Annotation sequence length = Long.MAX_VALUE + 0x9E // The start of the annotation sequence + ) + ) { + assertThrows(IonException.class, cursor::nextValue); + } + } + + @ParameterizedTest(name = "constructFromBytes={0}") + @ValueSource(booleans = {true, false}) + public void annotationWrapperLengthPlusCurrentIndexExceedsJavaLong(boolean constructFromBytes) { + try ( + IonCursorBinary cursor = initializeCursor( + constructFromBytes, + 0xE0, 0x01, 0x00, 0xEA, // IVM + 0xEE, // Annotation wrapper, variable length + 0x7F, 0x7F, 0x7F, 0x7F, 0x7F, 0x7F, 0x7F, 0x7F, 0xFF, // Annotation wrapper length = Long.MAX_VALUE + 0x81, // Annotation sequence length = 1 + 0x83, // SID 3 + 0x9E // The start of the wrapped value + ) + ) { + assertThrows(IonException.class, cursor::nextValue); + } + } }