From: Matt Caswell <matt@openssl.org>
Date: Fri, 24 Jun 2016 22:37:27 +0000 (+0100)
Subject: Convert memset calls to OPENSSL_cleanse
X-Git-Tag: OpenSSL_1_1_0-pre6~305
X-Git-Url: https://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff_plain;h=3ce2fdabe6e33952bf3011acf5b68107e6352603;ds=sidebyside

Convert memset calls to OPENSSL_cleanse

Ensure things really do get cleared when we intend them to.

Addresses an OCAP Audit issue.

Reviewed-by: Andy Polyakov <appro@openssl.org>
---

diff --git a/crypto/bn/bn_lib.c b/crypto/bn/bn_lib.c
index 90df3eee3b..b606cc9c32 100644
--- a/crypto/bn/bn_lib.c
+++ b/crypto/bn/bn_lib.c
@@ -445,7 +445,7 @@ void BN_clear(BIGNUM *a)
 {
     bn_check_top(a);
     if (a->d != NULL)
-        memset(a->d, 0, sizeof(*a->d) * a->dmax);
+        OPENSSL_cleanse(a->d, sizeof(*a->d) * a->dmax);
     a->top = 0;
     a->neg = 0;
 }
diff --git a/crypto/buffer/buffer.c b/crypto/buffer/buffer.c
index 7caa215092..6b0bd4a404 100644
--- a/crypto/buffer/buffer.c
+++ b/crypto/buffer/buffer.c
@@ -46,7 +46,6 @@ void BUF_MEM_free(BUF_MEM *a)
         return;
 
     if (a->data != NULL) {
-        memset(a->data, 0, (unsigned int)a->max);
         if (a->flags & BUF_MEM_FLAG_SECURE)
             OPENSSL_secure_free(a->data);
         else
diff --git a/crypto/evp/digest.c b/crypto/evp/digest.c
index c594a0a638..65eff7c8c1 100644
--- a/crypto/evp/digest.c
+++ b/crypto/evp/digest.c
@@ -36,7 +36,7 @@ int EVP_MD_CTX_reset(EVP_MD_CTX *ctx)
 #ifndef OPENSSL_NO_ENGINE
     ENGINE_finish(ctx->engine);
 #endif
-    memset(ctx, 0, sizeof(*ctx));
+    OPENSSL_cleanse(ctx, sizeof(*ctx));
 
     return 1;
 }
@@ -170,7 +170,7 @@ int EVP_DigestFinal_ex(EVP_MD_CTX *ctx, unsigned char *md, unsigned int *size)
         ctx->digest->cleanup(ctx);
         EVP_MD_CTX_set_flags(ctx, EVP_MD_CTX_FLAG_CLEANED);
     }
-    memset(ctx->md_data, 0, ctx->digest->ctx_size);
+    OPENSSL_cleanse(ctx->md_data, ctx->digest->ctx_size);
     return ret;
 }
 
diff --git a/crypto/include/internal/md32_common.h b/crypto/include/internal/md32_common.h
index 21133a37d7..6e4ce14e99 100644
--- a/crypto/include/internal/md32_common.h
+++ b/crypto/include/internal/md32_common.h
@@ -65,6 +65,8 @@
  *                                      <appro@fy.chalmers.se>
  */
 
+#include <openssl/crypto.h>
+
 #if !defined(DATA_ORDER_IS_BIG_ENDIAN) && !defined(DATA_ORDER_IS_LITTLE_ENDIAN)
 # error "DATA_ORDER must be defined!"
 #endif
@@ -276,6 +278,12 @@ int HASH_UPDATE(HASH_CTX *c, const void *data_, size_t len)
             data += n;
             len -= n;
             c->num = 0;
+            /*
+             * We use memset rather than OPENSSL_cleanse() here deliberately.
+             * Using OPENSSL_cleanse() here could be a performance issue. It
+             * will get properly cleansed on finalisation so this isn't a
+             * security problem.
+             */
             memset(p, 0, HASH_CBLOCK); /* keep it zeroed */
         } else {
             memcpy(p + n, data, len);
@@ -331,7 +339,7 @@ int HASH_FINAL(unsigned char *md, HASH_CTX *c)
     p -= HASH_CBLOCK;
     HASH_BLOCK_DATA_ORDER(c, p, 1);
     c->num = 0;
-    memset(p, 0, HASH_CBLOCK);
+    OPENSSL_cleanse(p, HASH_CBLOCK);
 
 #ifndef HASH_MAKE_STRING
 # error "HASH_MAKE_STRING must be defined!"
diff --git a/crypto/md2/md2_dgst.c b/crypto/md2/md2_dgst.c
index b43cd4f489..ff062fd472 100644
--- a/crypto/md2/md2_dgst.c
+++ b/crypto/md2/md2_dgst.c
@@ -168,6 +168,6 @@ int MD2_Final(unsigned char *md, MD2_CTX *c)
 
     for (i = 0; i < 16; i++)
         md[i] = (UCHAR) (p1[i] & 0xff);
-    memset(&c, 0, sizeof(c));
+    OPENSSL_cleanse(c, sizeof(*c));
     return 1;
 }
diff --git a/crypto/mem.c b/crypto/mem.c
index 6be14ab54a..02aa43a7ef 100644
--- a/crypto/mem.c
+++ b/crypto/mem.c
@@ -148,7 +148,7 @@ void *CRYPTO_clear_realloc(void *str, size_t old_len, size_t num,
 
     /* Can't shrink the buffer since memcpy below copies |old_len| bytes. */
     if (num < old_len) {
-        memset((char*)str + num, 0, old_len - num);
+        OPENSSL_cleanse((char*)str + num, old_len - num);
         return str;
     }
 
diff --git a/crypto/poly1305/poly1305.c b/crypto/poly1305/poly1305.c
index 55de19b7ed..eec4d67f0c 100644
--- a/crypto/poly1305/poly1305.c
+++ b/crypto/poly1305/poly1305.c
@@ -9,6 +9,7 @@
 
 #include <stdlib.h>
 #include <string.h>
+#include <openssl/crypto.h>
 
 #include "internal/poly1305.h"
 
@@ -545,7 +546,7 @@ void Poly1305_Final(POLY1305 *ctx, unsigned char mac[16])
     poly1305_emit(ctx->opaque, mac, ctx->nonce);
 
     /* zero out the state */
-    memset(ctx, 0, sizeof(*ctx));
+    OPENSSL_cleanse(ctx, sizeof(*ctx));
 }
 
 #ifdef SELFTEST
diff --git a/crypto/rand/rand_unix.c b/crypto/rand/rand_unix.c
index e231ecdb82..ecba2dc984 100644
--- a/crypto/rand/rand_unix.c
+++ b/crypto/rand/rand_unix.c
@@ -134,7 +134,7 @@ int RAND_poll(void)
         rnd >>= 8;
     }
     RAND_add(buf, sizeof(buf), ENTROPY_NEEDED);
-    memset(buf, 0, sizeof(buf));
+    OPENSSL_cleanse(buf, sizeof(buf));
 
     return 1;
 }
diff --git a/crypto/whrlpool/wp_dgst.c b/crypto/whrlpool/wp_dgst.c
index d852db6554..ed064244fa 100644
--- a/crypto/whrlpool/wp_dgst.c
+++ b/crypto/whrlpool/wp_dgst.c
@@ -60,6 +60,7 @@
  * input. This is done for performance.
  */
 
+#include <openssl/crypto.h>
 #include "wp_locl.h"
 #include <string.h>
 
@@ -245,7 +246,7 @@ int WHIRLPOOL_Final(unsigned char *md, WHIRLPOOL_CTX *c)
 
     if (md) {
         memcpy(md, c->H.c, WHIRLPOOL_DIGEST_LENGTH);
-        memset(c, 0, sizeof(*c));
+        OPENSSL_cleanse(c, sizeof(*c));
         return (1);
     }
     return (0);