Code review request for 7172149 ArrayIndexOutOfBoundsException from Signature.verify

[Jonathan Lu]

Hi Xuelei,

On 06/05/2012 12:17 PM, Xuelei Fan wrote:
> Hi Jonathan,
>
> The fix, including the copyright notice, looks good to us. Are you a
> committer of OpenJDK? Otherwise, I would like to help to integrate the
I'm not.
> fix into OpenJDK.
Please help me to integrate the fix into OpenJDK.

Thanks!
- Jonathan
>
> Thanks,
> Xuelei
>
> On 6/5/2012 8:52 AM, Brad Wetmore wrote:
>> 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
>>>>>>>