Code review request, 7200295 CertificateRequest message is wrapping when using large numbers of Certs

Christophe Ravel christophe.ravel at oracle.com
Thu Sep 27 20:35:08 UTC 2012


Thanks.

Christophe.

On Sep 26, 2012, at 8:13 PM, Xuelei Fan <Xuelei.Fan at oracle.COM> wrote:

> A new regression test was added.
> 
> http://cr.openjdk.java.net./~xuelei/7200295/webrev.01/
> 
> Thanks,
> Xuelei
> 
> On 9/26/2012 4:53 AM, Christophe Ravel wrote:
>> Hi Andrew,
>> 
>> You need to add a regression test for this fix.
>> 
>> Regards,
>> Christophe.
>> 
>> On Sep 24, 2012, at 7:01 PM, Xuelei Fan <Xuelei.Fan at oracle.COM> wrote:
>> 
>>> On 9/25/2012 9:23 AM, Brad Wetmore wrote:
>>>> Are there situations where we might overflow the int?
>>>> 
>>> Yes, it is possible for many integer add operations. As 2^32 is a lot
>>> bigger than 2^24 (the biggest number TLS protocol allows), I'm not
>>> worried too much about int32 overflow.
>>> 
>>> Integer overflow checking would make the code ugly. For example,
>>> normally, we do add operations as:
>>>  int result = 1 + len + anotherLen;
>>> 
>>> if we want to check overflow, the code would look like:
>>>  int result = 1;
>>>  if (result > Integer.MAX_VALUE - len) {
>>>      result += len;
>>>  } else {
>>>      // overflow
>>>  }
>>> 
>>>  // the same for anotherLen
>>> 
>>> I did not think it is necessary.
>>> 
>>>> For example, in CertificateRequest.messageLength()
>>>> 
>>>>       for (int i = 0; i < authorities.length; i++) {
>>>>           len += authorities[i].length();
>>>>       }
>>>> 
>>>> What if len overflows?
>>>> 
>>>> Also, all of these field's callers are overflow-1?
>>>> 
>>> I'm not sure I get your point. In RFC5246, exception session ID, other
>>> variable length is one of 2^8-1, 2^16-1 or 2^24 -1.
>>> 
>>> Xuelei
>>> 
>>>> Brad
>>>> 
>>>> 
>>>> 
>>>> 
>>>> On 9/23/2012 7:42 PM, Xuelei Fan wrote:
>>>>> Hi,
>>>>> 
>>>>> Please review the update to check output filed length overflow in TLS
>>>>> handshaking.
>>>>> 
>>>>> bug   : http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7200295
>>>>> webrev: http://cr.openjdk.java.net/~xuelei/7200295/webrev.00/
>>>>> 
>>>>> The cause of the bug is that for 8, 16, 24 bits length-variable fields,
>>>>> before put the bytes into the fields, we do not check that the length of
>>>>> the bytes is less than the capabilities of the field.
>>>>> 
>>>>> Thanks,
>>>>> Xuelei
>>>>> 
>>> 
>> 
>> Christophe Ravel | Principal Member of Technical Staff | +1.650.506.2162
>> Oracle Java SQE - Security
>> 4220 Network Circle, B160A, Santa Clara, CA 
>> 
> 

Christophe Ravel | Principal Member of Technical Staff | +1.650.506.2162
Oracle Java SQE - Security
4220 Network Circle, B160A, Santa Clara, CA 




More information about the security-dev mailing list