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.

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



Reply via email to