commit 6f5f49f33d88f446f7f690bf991e835d5137af6c
author Adam Langley <agl@google.com> Fri Nov 18 11:05:00 2016 -0800
committer Adam Langley <agl@google.com> Fri Nov 18 22:01:38 2016 +0000
tree e8945547cd1a8e1e2df3c4f04ee15749203b1a1a
parent cfa08c3b7740275ef3994ff07a2c7f4639c88ccb

Flush TLS 1.3 certificate extensions.

(Otherwise we end up touching potentially unwound stack.)

I looked into why our builders didn't catch this and it appears that, at
least with Clang 3.7, ASAN doesn't notice this. Perhaps Clang at that
version is being lazy about destructing the scoped CBB and so doesn't
actually go wrong.

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

ssl/tls13_both.c

diff --git a/ssl/tls13_both.c b/ssl/tls13_both.c
index 9bf169d..17f7161 100644
--- a/ssl/tls13_both.c
+++ b/ssl/tls13_both.c
@@ -457,7 +457,8 @@
     if (!CBB_add_u16(&extensions, TLSEXT_TYPE_certificate_timestamp) ||
         !CBB_add_u16_length_prefixed(&extensions, &contents) ||
         !CBB_add_bytes(&contents, ssl->ctx->signed_cert_timestamp_list,
-                       ssl->ctx->signed_cert_timestamp_list_length)) {
+                       ssl->ctx->signed_cert_timestamp_list_length) ||
+        !CBB_flush(&extensions)) {
       OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
       goto err;
     }
@@ -471,7 +472,8 @@
         !CBB_add_u8(&contents, TLSEXT_STATUSTYPE_ocsp) ||
         !CBB_add_u24_length_prefixed(&contents, &ocsp_response) ||
         !CBB_add_bytes(&ocsp_response, ssl->ctx->ocsp_response,
-                       ssl->ctx->ocsp_response_length)) {
+                       ssl->ctx->ocsp_response_length) ||
+        !CBB_flush(&extensions)) {
       OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
       goto err;
     }