Code review request for 7172149 ArrayIndexOutOfBoundsException from Signature.verify

[Jonathan Lu]

Hi Brad,

I think I've found a better solution after reading into other modules, 
here's the refined patch
http://cr.openjdk.java.net/~luchsh/7172149_2/

On 05/30/2012 08:54 AM, Brad Wetmore wrote:
> I think it is worth exploring what other parts of the code do in this 
> case.  It seems to me this is going to be a lot more involved than 
> just Signatures.  (InputStreams, etc)
>
> Brad
>
>
>
> On 5/29/2012 4:26 PM, 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.

The new patch is quite straightforward, I do not there is any 
requirement for a comment now :)
Does it make sense?

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

Thanks!
- Jonathan