Fix leak if X509_STORE_CTX_init is called on a previously initialized context

This wasn't possible when X509_STORE_CTX was stack-allocated because
X509_STORE_CTX_init needed to account for an uninitialized struct. But
now it is always initialized, so we can avoid this footgun. This also
matches what OpenSSL does nowadays.

Change-Id: I266be58204b8cd374fa4896c1c66a35ffaa762ea
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64141
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit 1685bd140f6eeb6939c73756be70c888dde32c5e)

Author: davidben

crypto/x509/x509_test.cc

@@ -3888,6 +3888,18 @@ TEST(X509Test, NullStore) {
   EXPECT_FALSE(X509_STORE_CTX_init(ctx.get(), nullptr, leaf.get(), nullptr));
 }
 
+TEST(X509Test, StoreCtxReuse) {
+  bssl::UniquePtr<X509> leaf(CertFromPEM(kLeafPEM));
+  ASSERT_TRUE(leaf);
+  bssl::UniquePtr<X509_STORE> store(X509_STORE_new());
+  ASSERT_TRUE(store);
+  bssl::UniquePtr<X509_STORE_CTX> ctx(X509_STORE_CTX_new());
+  ASSERT_TRUE(ctx);
+  ASSERT_TRUE(X509_STORE_CTX_init(ctx.get(), store.get(), leaf.get(), nullptr));
+  // Re-initializing |ctx| should not leak memory.
+  ASSERT_TRUE(X509_STORE_CTX_init(ctx.get(), store.get(), leaf.get(), nullptr));
+}
+
 TEST(X509Test, BasicConstraints) {
   const uint32_t kFlagMask = EXFLAG_CA | EXFLAG_BCONS | EXFLAG_INVALID;
 

crypto/x509/x509_vfy.c

@@ -1672,13 +1672,8 @@ void X509_STORE_CTX_free(X509_STORE_CTX *ctx) {
 
 int X509_STORE_CTX_init(X509_STORE_CTX *ctx, X509_STORE *store, X509 *x509,
                         STACK_OF(X509) *chain) {
-  // TODO(davidben): This is a remnant of when |X509_STORE_CTX| was a
-  // stack-allocatable function. Now that it is heap-allocated, we don't need to
-  // worry about uninitialized memory in |ctx|. Move the memset to
-  // |X509_STORE_CTX_cleanup| and call |X509_STORE_CTX_cleanup| here so callers
-  // don't leak memory when re-initializing a previously initialized
-  // |X509_STORE_CTX|.
-  OPENSSL_memset(ctx, 0, sizeof(X509_STORE_CTX));
+  X509_STORE_CTX_cleanup(ctx);
+
   ctx->ctx = store;
   ctx->cert = x509;
   ctx->untrusted = chain;
@@ -1805,7 +1800,7 @@ void X509_STORE_CTX_cleanup(X509_STORE_CTX *ctx) {
   sk_X509_pop_free(ctx->chain, X509_free);
   ctx->chain = NULL;
   CRYPTO_free_ex_data(&g_ex_data_class, ctx, &(ctx->ex_data));
-  OPENSSL_memset(&ctx->ex_data, 0, sizeof(CRYPTO_EX_DATA));
+  OPENSSL_memset(ctx, 0, sizeof(X509_STORE_CTX));
 }
 
 void X509_STORE_CTX_set_depth(X509_STORE_CTX *ctx, int depth) {