commit 9c33ae85621ef8e00a42309b5101e0bedd02b816
author David Benjamin <davidben@google.com> Sun Jan 08 06:04:43 2017 -0500
committer Adam Langley <agl@google.com> Mon Jan 09 03:37:19 2017 +0000
tree 73e8c85aecfee2edf162e7dd3ac864ec0214b6fc
parent 6add9f172c88a81931d22aeaf18ce4553588d35e

Fix TLS 1.3 NewSessionTicket processing.

08b65f4e31f7eb63bad0e903e9f191bcc699eaca introduced a memory leak and
also got enums confused. Also fix a codepath that was missing an error
code.

Thanks to OSS-Fuzz which appears to have found it in a matter of hours.

Change-Id: Ia9e926c28a01daab3e6154d363d0acda91209a22
Reviewed-on: https://boringssl-review.googlesource.com/13104
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>

ssl/tls13_client.c

diff --git a/ssl/tls13_client.c b/ssl/tls13_client.c
index 1d8bc54..6f2bb21 100644
--- a/ssl/tls13_client.c
+++ b/ssl/tls13_client.c
@@ -638,6 +638,7 @@
 }
 
 int tls13_process_new_session_ticket(SSL *ssl) {
+  int ret = 0;
   SSL_SESSION *session =
       SSL_SESSION_dup(ssl->s3->established_session,
                       SSL_SESSION_INCLUDE_NONAUTH);
@@ -655,10 +656,9 @@
       !CBS_stow(&ticket, &session->tlsext_tick, &session->tlsext_ticklen) ||
       !CBS_get_u16_length_prefixed(&cbs, &extensions) ||
       CBS_len(&cbs) != 0) {
-    SSL_SESSION_free(session);
     ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
     OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
-    return 0;
+    goto err;
   }
 
   /* Parse out the extensions. */
@@ -674,14 +674,15 @@
                             OPENSSL_ARRAY_SIZE(ext_types),
                             1 /* ignore unknown */)) {
     ssl3_send_alert(ssl, SSL3_AL_FATAL, alert);
-    return ssl_hs_error;
+    goto err;
   }
 
   if (have_early_data_info) {
     if (!CBS_get_u32(&early_data_info, &session->ticket_max_early_data) ||
         CBS_len(&early_data_info) != 0) {
       ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
-      return ssl_hs_error;
+      OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
+      goto err;
     }
   }
 
@@ -691,11 +692,14 @@
   if (ssl->ctx->new_session_cb != NULL &&
       ssl->ctx->new_session_cb(ssl, session)) {
     /* |new_session_cb|'s return value signals that it took ownership. */
-    return 1;
+    session = NULL;
   }
 
+  ret = 1;
+
+err:
   SSL_SESSION_free(session);
-  return 1;
+  return ret;
 }
 
 void ssl_clear_tls13_state(SSL_HANDSHAKE *hs) {