commit 2b07fa4b22198ac02e0cee8f37f3337c3dba91bc
author David Benjamin <davidben@google.com> Wed Mar 02 00:23:57 2016 -0500
committer Adam Langley <agl@google.com> Wed Mar 02 15:49:30 2016 +0000
tree 6eaee3d04cb37a3f5c5b76e1f11fbf15f2331a5c
parent ff3a1498dae79e7fa1cf8d93f735f6fdae49e286

Fix a memory leak in an error path.

Found by libFuzzer combined with some experimental unsafe-fuzzer-mode patches
(to be uploaded once I've cleaned them up a bit) to disable all those pesky
cryptographic checks in the protocol.

Change-Id: I9153164fa56a0c2262c4740a3236c2b49a596b1b
Reviewed-on: https://boringssl-review.googlesource.com/7282
Reviewed-by: Adam Langley <agl@google.com>

ssl/s3_clnt.c

diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index 09e527a..bbbaccd 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -1764,6 +1764,7 @@
   return ssl_do_write(ssl);
 
 err:
+  CBB_cleanup(&cbb);
   if (pms != NULL) {
     OPENSSL_cleanse(pms, pms_len);
     OPENSSL_free(pms);

ssl/test/runner/common.go

diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index f2ef360..e7e4213 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -424,6 +424,10 @@
 	// to be wrong.
 	InvalidSKXCurve bool
 
+	// InvalidECDHPoint, if true, causes the ECC points in
+	// ServerKeyExchange or ClientKeyExchange messages to be invalid.
+	InvalidECDHPoint bool
+
 	// BadECDSAR controls ways in which the 'r' value of an ECDSA signature
 	// can be invalid.
 	BadECDSAR BadValue

ssl/test/runner/key_agreement.go

diff --git a/ssl/test/runner/key_agreement.go b/ssl/test/runner/key_agreement.go
index 9ecd2e5..54aa3d3 100644
--- a/ssl/test/runner/key_agreement.go
+++ b/ssl/test/runner/key_agreement.go
@@ -566,6 +566,9 @@
 	}
 	serverECDHParams[3] = byte(len(publicKey))
 	copy(serverECDHParams[4:], publicKey)
+	if config.Bugs.InvalidECDHPoint {
+		serverECDHParams[4] ^= 0xff
+	}
 
 	return ka.auth.signParameters(config, cert, clientHello, hello, serverECDHParams)
 }
@@ -622,6 +625,9 @@
 	ckx.ciphertext = make([]byte, 1+len(publicKey))
 	ckx.ciphertext[0] = byte(len(publicKey))
 	copy(ckx.ciphertext[1:], publicKey)
+	if config.Bugs.InvalidECDHPoint {
+		ckx.ciphertext[1] ^= 0xff
+	}
 
 	return preMasterSecret, ckx, nil
 }

ssl/test/runner/runner.go

diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 57f1089..68bfd56 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -2106,6 +2106,31 @@
 			resumeConfig:  &Config{},
 			resumeSession: true,
 		},
+		{
+			name: "InvalidECDHPoint-Client",
+			config: Config{
+				CipherSuites:     []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
+				CurvePreferences: []CurveID{CurveP256},
+				Bugs: ProtocolBugs{
+					InvalidECDHPoint: true,
+				},
+			},
+			shouldFail:    true,
+			expectedError: ":INVALID_ENCODING:",
+		},
+		{
+			testType: serverTest,
+			name:     "InvalidECDHPoint-Server",
+			config: Config{
+				CipherSuites:     []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
+				CurvePreferences: []CurveID{CurveP256},
+				Bugs: ProtocolBugs{
+					InvalidECDHPoint: true,
+				},
+			},
+			shouldFail:    true,
+			expectedError: ":INVALID_ENCODING:",
+		},
 	}
 	testCases = append(testCases, basicTests...)
 }