Code review request for 7172149 ArrayIndexOutOfBoundsException from Signature.verify

[Jonathan Lu]

Hello Xuelei,

What do you think of the updated patch? any comments?
http://cr.openjdk.java.net/~luchsh/7172149_2/

Thanks
  - Jonathan

On 05/30/2012 07:26 AM, Xuelei Fan wrote:
> On 5/29/2012 3:11 PM, Jonathan Lu wrote:
>> Hi Xuelei,
>>
>> Thanks for review!
>>
>> On 05/29/2012 02:45 PM, Xuelei Fan wrote:
>>> That's an interesting topic.  From my understand, the length of an array
>>> is of type "int".  So normally, the (offset + length) should not be
>>> great than integer.max_value.  Of course, Hostile or improper code are
>>> not of the case.
>>>
>>> What's interesting to me is that may be when we do additive operation
>>> for two "int" values, we may have to convert it to "long" in case of any
>>> overflow strictly.  We are luck here because we have "long" type. But
>>> what about the additive operation for two "long" values
>> I think this issue is special, since it is about index value of Java
>> arrays, which is limited to smaller than Integer.MAX_VALUE according to
>> Java language specification, not other general conditions of comparing
>> integer or long values.
>>
>>> Jonathan, do you run into the problem in real world?
>> For now I am not quiet sure of whether it is from a real world problem,
>> but this problem does exhibit some weakness or behavior differences, right?
>>
> Yes, it is an improvement.  Would you please add a comment about why
> convert it to "long", and update the copyright year to 2012? Otherwise,
> looks fine to me for JDK 8.
>
> Thanks&   Regards,
> Xuelei
>
>> Thanks&  regards
>> -Jonathan
>>
>>> Thanks&   Regards,
>>> Xuelei
>>>
>>> On 5/29/2012 1:53 PM, Jonathan Lu wrote:
>>>> Hi Security-dev,
>>>>
>>>> Here's a patch for bug7172149, could anybody please help to take a look?
>>>> http://cr.openjdk.java.net/~luchsh/7172149/
>>>>
>>>> The problem is that the range check in Signature.verify(byte[], int,
>>>> int) uses integer value to check whether (offset + length) is greater
>>>> than signature.length, but if (offset + length) overflows the check will
>>>> fail and ArrayIndexOutOfBoundsException will be thrown instead of
>>>> IllegalArgumentException.My proposed solution is to make a  conversion
>>>> to long in the if block.
>>>>
>>>> Thanks!
>>>> - Jonathan
>>>>