Re: Code review request for 7172149 ArrayIndexOutOfBoundsException from Signature.verify

2012-06-05 Thread Jonathan Lu
Hi Xuelei, On 06/05/2012 12:17 PM, Xuelei Fan wrote: 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 I'm not. fix into OpenJDK. Please help me to integrate the fix into OpenJDK. Thanks!

Re: Code review request for 7172149 ArrayIndexOutOfBoundsException from Signature.verify

2012-06-04 Thread 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 n

Re: Code review request for 7172149 ArrayIndexOutOfBoundsException from Signature.verify

2012-06-04 Thread Brad Wetmore
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 th

Re: Code review request for 7172149 ArrayIndexOutOfBoundsException from Signature.verify

2012-05-31 Thread Jonathan Lu
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: Th

Re: Code review request for 7172149 ArrayIndexOutOfBoundsException from Signature.verify

2012-05-30 Thread 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 i

Re: Code review request for 7172149 ArrayIndexOutOfBoundsException from Signature.verify

2012-05-29 Thread Brad Wetmore
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

Re: Code review request for 7172149 ArrayIndexOutOfBoundsException from Signature.verify

2012-05-29 Thread 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_v

Re: Code review request for 7172149 ArrayIndexOutOfBoundsException from Signature.verify

2012-05-29 Thread Jonathan Lu
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

Re: Code review request for 7172149 ArrayIndexOutOfBoundsException from Signature.verify

2012-05-28 Thread Xuelei Fan
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

Code review request for 7172149 ArrayIndexOutOfBoundsException from Signature.verify

2012-05-28 Thread Jonathan Lu
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