Code review request for 7172149 ArrayIndexOutOfBoundsException from Signature.verify

[Brad Wetmore]

Looks good to me, however, we are still checking on the copyright notice 
in the test.  I think what you have is ok, but want to run it by one 
other person.  Hope to hear back soon.

The bug presents an interesting and obvious problem when you think of 
it.  Wonder how many things we have like that elsewhere.  Another thing 
to keep in the back on my mind for future codereviews.

Thanks,

Brad



On 5/31/2012 11:25 PM, Jonathan Lu wrote:
> 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
>>>>>
>