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