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