From 3831173740ebdb35bc9371a3886dcaa1ce0b9227 Mon Sep 17 00:00:00 2001 From: Adam Langley Date: Thu, 16 Oct 2014 19:04:35 -0700 Subject: [PATCH] Fix memory leak when decoding corrupt tickets. This is CVE-2014-3567 from upstream. See https://www.openssl.org/news/secadv_20141015.txt Change-Id: I9aad422bf1b8055cb251c7ff9346cf47a448a815 Reviewed-on: https://boringssl-review.googlesource.com/1970 Reviewed-by: David Benjamin Reviewed-by: Adam Langley --- ssl/t1_lib.c | 3 +++ ssl/test/runner/common.go | 8 ++++++++ ssl/test/runner/handshake_client.go | 17 ++++++++++++++++- ssl/test/runner/runner.go | 29 +++++++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 1 deletion(-) diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index a14ce5ad..12c67b95 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -2379,7 +2379,10 @@ static int tls_decrypt_ticket(SSL *s, const unsigned char *etick, int eticklen, HMAC_Final(&hctx, tick_hmac, NULL); HMAC_CTX_cleanup(&hctx); if (CRYPTO_memcmp(tick_hmac, etick + eticklen, mlen)) + { + EVP_CIPHER_CTX_cleanup(&ctx); return 2; + } /* Attempt to decrypt session data */ /* Move p after IV to start of encrypted ticket, update length */ p = etick + 16 + EVP_CIPHER_CTX_iv_length(&ctx); diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go index cf244bc2..8b2c7503 100644 --- a/ssl/test/runner/common.go +++ b/ssl/test/runner/common.go @@ -464,6 +464,14 @@ type ProtocolBugs struct { // AllowSessionVersionMismatch causes the server to resume sessions // regardless of the version associated with the session. AllowSessionVersionMismatch bool + + // CorruptTicket causes a client to corrupt a session ticket before + // sending it in a resume handshake. + CorruptTicket bool + + // OversizedSessionId causes the session id that is sent with a ticket + // resumption attempt to be too large (33 bytes). + OversizedSessionId bool } func (c *Config) serverInit() { diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go index d78e767b..f4cadc26 100644 --- a/ssl/test/runner/handshake_client.go +++ b/ssl/test/runner/handshake_client.go @@ -148,10 +148,25 @@ NextCipherSuite: if session != nil { hello.sessionTicket = session.sessionTicket + if c.config.Bugs.CorruptTicket { + hello.sessionTicket = make([]byte, len(session.sessionTicket)) + copy(hello.sessionTicket, session.sessionTicket) + if len(hello.sessionTicket) > 0 { + offset := 40 + if offset > len(hello.sessionTicket) { + offset = len(hello.sessionTicket) - 1 + } + hello.sessionTicket[offset] ^= 0x40 + } + } // A random session ID is used to detect when the // server accepted the ticket and is resuming a session // (see RFC 5077). - hello.sessionId = make([]byte, 16) + sessionIdLen := 16 + if c.config.Bugs.OversizedSessionId { + sessionIdLen = 33 + } + hello.sessionId = make([]byte, sessionIdLen) if _, err := io.ReadFull(c.config.rand(), hello.sessionId); err != nil { c.sendAlert(alertInternalError) return errors.New("tls: short read from Rand: " + err.Error()) diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index 323f43fa..b4c2e612 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -649,6 +649,10 @@ func openSocketPair() (shimEnd *os.File, conn net.Conn) { } func runTest(test *testCase, buildDir string) error { + if !test.shouldFail && (len(test.expectedError) > 0 || len(test.expectedLocalError) > 0) { + panic("Error expected without shouldFail in " + test.name) + } + shimEnd, conn := openSocketPair() shimEndResume, connResume := openSocketPair() @@ -1542,6 +1546,31 @@ func addExtensionTests() { expectedNextProtoType: alpn, resumeSession: true, }) + // Resume with a corrupt ticket. + testCases = append(testCases, testCase{ + testType: serverTest, + name: "CorruptTicket", + config: Config{ + Bugs: ProtocolBugs{ + CorruptTicket: true, + }, + }, + resumeSession: true, + flags: []string{"-expect-session-miss"}, + }) + // Resume with an oversized session id. + testCases = append(testCases, testCase{ + testType: serverTest, + name: "OversizedSessionId", + config: Config{ + Bugs: ProtocolBugs{ + OversizedSessionId: true, + }, + }, + resumeSession: true, + shouldFail: true, + expectedError: ":DECODE_ERROR:", + }) } func addResumptionVersionTests() {