Code review request for 7172149 ArrayIndexOutOfBoundsException from Signature.verify

[Xuelei Fan]

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