Code review request for 7172149 ArrayIndexOutOfBoundsException from Signature.verify

[Xuelei Fan]

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
fix into OpenJDK.

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
>>>>>>
>>