You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The IonParser::nextToken() method relies on the IonReader implementations to retrieve the next token. Those IonReader implementations are provided by the upstream Amazon Java-Ion package and some of the code in those IonReader implementations does mention that if the provided data is malformed, it could throw IndexOutOfBoundsException and that is not handled because it would sacrifice performance. And IonParser::nextToken() fails to handle them nor check if the input is malformed. This results in an unexpected IndexOutOfBoundsException being thrown to the user.
privateintreadVarInt_1_0(intfirstByte) {
intcurrentByte = firstByte;
intsign = (currentByte & VAR_INT_SIGN_BITMASK) == 0 ? 1 : -1;
intresult = currentByte & LOWER_SIX_BITS_BITMASK;
while ((currentByte & HIGHEST_BIT_BITMASK) == 0) {
// Note: if the varInt is malformed such that it extends beyond the declared length of the value *and*// beyond the end of the buffer, this will result in IndexOutOfBoundsException because only the declared// value length has been filled. Preventing this is simple: if (peekIndex >= valueMarker.endIndex) throw// new IonException(); However, we choose not to perform that check here because it is not worth sacrificing// performance in this inner-loop code in order to throw one type of exception over another in case of// malformed data.currentByte = buffer[(int)(peekIndex++)];
result = (result << VALUE_BITS_PER_VARUINT_BYTE) | (currentByte & LOWER_SEVEN_BITS_BITMASK);
}
returnresult * sign;
}
privatelonguncheckedReadVarUInt_1_0(bytecurrentByte) {
longresult = currentByte & LOWER_SEVEN_BITS_BITMASK;
do {
// Note: if the varUInt is malformed such that it extends beyond the declared length of the value *and*// beyond the end of the buffer, this will result in IndexOutOfBoundsException because only the declared// value length has been filled. Preventing this is simple: if (peekIndex >= limit) throw// new IonException(); However, we choose not to perform that check here because it is not worth sacrificing// performance in this inner-loop code in order to throw one type of exception over another in case of// malformed data.currentByte = buffer[(int) (peekIndex++)];
result = (result << VALUE_BITS_PER_VARUINT_BYTE) | (currentByte & LOWER_SEVEN_BITS_BITMASK);
} while (currentByte >= 0);
returnresult;
}
The simplest fix is to catch the IndexOutOfBoundsException and wrap it with the JsonParseException. A better way may be adding some checking before the upstream call to ensure malformed data is detected and exit before calling those upstream methods.
Lol. "Doing the right thing would be bit slower than letting things fail in bad ways" is certainly one way to approach things! :-D :-D :-D
Ok, yeah, If so, need to work around such sub-standard implementation. Will look at PR next.
cowtowncoder
changed the title
IndexOutOfBoundsException thrown from different IonReader implementations are not handled IndexOutOfBoundsException thrown by IonReader implementations are not handled
Dec 16, 2023
The IonParser::nextToken() method relies on the IonReader implementations to retrieve the next token. Those IonReader implementations are provided by the upstream Amazon Java-Ion package and some of the code in those IonReader implementations does mention that if the provided data is malformed, it could throw
IndexOutOfBoundsException
and that is not handled because it would sacrifice performance. And IonParser::nextToken() fails to handle them nor check if the input is malformed. This results in an unexpectedIndexOutOfBoundsException
being thrown to the user.Below are two sample code that mentioned the possible throwing of
IndexOutOfBoundsException
from the upstream IonCursorBinary::uncheckedReadVarUInt_1_0(byte) method and IonReaderContinuableCoreBinary::readVarInt_1_0 method.The simplest fix is to catch the
IndexOutOfBoundsException
and wrap it with theJsonParseException
. A better way may be adding some checking before the upstream call to ensure malformed data is detected and exit before calling those upstream methods.We found this issue by OSS-Fuzz and it is reported in https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=65062 and https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=65083.
The text was updated successfully, but these errors were encountered: