commit c0e15d1d9d99bf5f603413e30ea5b6abd87ef9c1
author Martin Kreichgauer <martinkr@google.com> Fri Aug 18 14:24:36 2017 -0700
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Wed Sep 06 19:22:46 2017 +0000
tree b11349d143f8b0acf428fe9fb51ca505a1526309
parent a23b68f564fc412bd23d3f46b15a2bcd4825e480

Zero memory in |OPENSSL_free|.

Allocations by |OPENSSL_malloc| are prefixed with their length.
|OPENSSL_free| zeros the allocation before calling free(), eliminating
the need for a separate call to |OPENSSL_cleanse| for sensitive data.

This change will be followed up by the cleanup in
https://boringssl-review.googlesource.com/c/boringssl/+/19824.

Change-Id: Ie272f07e9248d7d78af9aea81dacec0fdb7484c4
Reviewed-on: https://boringssl-review.googlesource.com/19544
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

ssl/ssl_buffer.cc

diff --git a/ssl/ssl_buffer.cc b/ssl/ssl_buffer.cc
index d74278e..b424138 100644
--- a/ssl/ssl_buffer.cc
+++ b/ssl/ssl_buffer.cc
@@ -50,7 +50,11 @@
   }
 
   // Add up to |SSL3_ALIGN_PAYLOAD| - 1 bytes of slack for alignment.
-  uint8_t *new_buf = (uint8_t *)OPENSSL_malloc(cap + SSL3_ALIGN_PAYLOAD - 1);
+  //
+  // Since this buffer gets allocated quite frequently and doesn't contain any
+  // sensitive data, we allocate with malloc rather than |OPENSSL_malloc| and
+  // avoid zeroing on free.
+  uint8_t *new_buf = (uint8_t *)malloc(cap + SSL3_ALIGN_PAYLOAD - 1);
   if (new_buf == NULL) {
     OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
     return 0;
@@ -62,7 +66,7 @@
 
   if (buf->buf != NULL) {
     OPENSSL_memcpy(new_buf + new_offset, buf->buf + buf->offset, buf->len);
-    OPENSSL_free(buf->buf);
+    free(buf->buf);  // Allocated with malloc().
   }
 
   buf->buf = new_buf;
@@ -81,7 +85,7 @@
 }
 
 static void clear_buffer(SSL3_BUFFER *buf) {
-  OPENSSL_free(buf->buf);
+  free(buf->buf);  // Allocated with malloc().
   OPENSSL_memset(buf, 0, sizeof(SSL3_BUFFER));
 }
 

ssl/ssl_cipher.cc

diff --git a/ssl/ssl_cipher.cc b/ssl/ssl_cipher.cc
index e7a81a3..78cf60d 100644
--- a/ssl/ssl_cipher.cc
+++ b/ssl/ssl_cipher.cc
@@ -1337,11 +1337,15 @@
     goto err;
   }
   pref_list->ciphers = cipherstack;
-  pref_list->in_group_flags = (uint8_t *)OPENSSL_malloc(num_in_group_flags);
-  if (!pref_list->in_group_flags) {
-    goto err;
+  pref_list->in_group_flags = NULL;
+  if (num_in_group_flags) {
+    pref_list->in_group_flags = (uint8_t *)OPENSSL_malloc(num_in_group_flags);
+    if (!pref_list->in_group_flags) {
+      goto err;
+    }
+    OPENSSL_memcpy(pref_list->in_group_flags, in_group_flags,
+                   num_in_group_flags);
   }
-  OPENSSL_memcpy(pref_list->in_group_flags, in_group_flags, num_in_group_flags);
   OPENSSL_free(in_group_flags);
   in_group_flags = NULL;
   if (*out_cipher_list != NULL) {