On Thu, 20 Apr 2023 18:53:53 GMT, Roger Riggs <rri...@openjdk.org> wrote:

>> This issue was reported by: Yakov Shafranovich 
>> ([yako...@amazon.com](mailto:yako...@amazon.com))
>> 
>> Currently, `ObjectInputStream::readObject()` doesn't explicitly checks for a 
>> negative array length in the deserialization stream. Instead it calls 
>> `j.l.r.Array::newInstance(..)` with the negative length which results in a 
>> `NegativeArraySizeException`. NegativeArraySizeException is an unchecked 
>> exception which is neither declared in the signature of 
>> `ObjectInputStream::readObject()` nor mentioned in its API specification. It 
>> is therefore not obvious for users of `ObjectInputStream::readObject()` that 
>> they may have to handle `NegativeArraySizeException`s. It would therefor be 
>> better if a negative array length in the deserialization stream would be 
>> automatically wrapped in an `InvalidClassException` which is a checked 
>> exception (derived from `IOException` via `ObjectStreamException`) and 
>> declared in the signature of `ObjectInputStream::readObject()`.
>> 
>> If we do the negative array length check in 
>> `ObjectInputStream::readObject()` before filtering, this will then also fix 
>> `ObjectInputFilter.FilterInfo::arrayLength()` which is defined as:
>> 
>> Returns:
>> the non-negative number of array elements when deserializing an array of the 
>> class, otherwise -1
>> 
>> but currently returns a negative value if the array length is negative.
>
> test/jdk/java/io/ObjectInputStream/NegativeArraySizeTest.java line 59:
> 
>> 57:             if (serializedData[i] == 0x78) {
>> 58:                 firstPos = i;
>> 59:                 break;
> 
> Move setting the length code (lines 64-67) here and return immediately.

Fixed.

> test/jdk/java/io/ObjectInputStream/NegativeArraySizeTest.java line 62:
> 
>> 60:             }
>> 61:         }
>> 62: 
> 
> The test should fail if it falls through without finding the BLOCKDATA. Just 
> a double guard against the unexpected.

Fixed.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/13540#discussion_r1175180242
PR Review Comment: https://git.openjdk.org/jdk/pull/13540#discussion_r1175180425

Reply via email to