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

[Xuelei Fan]

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