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
