Reject XOF digests in DH_compute_key_hashed

DH_compute_key_hashed passes an uninitialized out_len to EVP_Digest.
For XOF digests, EVP_DigestFinalXOF interprets *out_size as an input length,
which can cause an out-of-bounds write to the caller-provided buffer.
Reject XOF digests and initialize out_len to the fixed digest size.

We would like to thank Joshua Rogers (https://joshua.hu/) of AISLE Research Team (https://aisle.com/) for reporting this issue.

Author: dkostic

crypto/dh_extra/dh_test.cc

@@ -67,6 +67,7 @@
 #include <openssl/bytestring.h>
 #include <openssl/crypto.h>
 #include <openssl/dh.h>
+#include <openssl/digest.h>
 #include <openssl/err.h>
 #include <openssl/mem.h>
 #include <openssl/nid.h>
@@ -76,6 +77,29 @@
 #include "openssl/pem.h"
 
 
+TEST(DHTest, ComputeKeyHashedRejectsXOF) {
+  bssl::UniquePtr<DH> a(DH_new_by_nid(NID_ffdhe2048));
+  ASSERT_TRUE(a);
+  bssl::UniquePtr<DH> b(DHparams_dup(a.get()));
+  ASSERT_TRUE(b);
+  ASSERT_TRUE(DH_generate_key(a.get()));
+  ASSERT_TRUE(DH_generate_key(b.get()));
+
+  uint8_t out[64];
+  size_t out_len = 0;
+
+  // XOF digests should be rejected.
+  EXPECT_FALSE(DH_compute_key_hashed(a.get(), out, &out_len, sizeof(out),
+                                     DH_get0_pub_key(b.get()), EVP_shake128()));
+  EXPECT_FALSE(DH_compute_key_hashed(a.get(), out, &out_len, sizeof(out),
+                                     DH_get0_pub_key(b.get()), EVP_shake256()));
+
+  // Non-XOF digests should succeed.
+  EXPECT_TRUE(DH_compute_key_hashed(a.get(), out, &out_len, sizeof(out),
+                                    DH_get0_pub_key(b.get()), EVP_sha256()));
+  EXPECT_EQ(out_len, 32u);
+}
+
 TEST(DHTest, Basic) {
   bssl::UniquePtr<DH> a(DH_new());
   ASSERT_TRUE(a);

crypto/fipsmodule/dh/dh.c

@@ -55,6 +55,7 @@
  * [including the GNU Public Licence.] */
 
 #include <openssl/dh.h>
+#include <limits.h>
 
 #include <string.h>
 
@@ -447,7 +448,11 @@ int DH_compute_key_hashed(DH *dh, uint8_t *out, size_t *out_len,
   *out_len = SIZE_MAX;
 
   const size_t digest_len = EVP_MD_size(digest);
-  if (digest_len > max_out_len) {
+  if (digest_len == 0 || digest_len > max_out_len
+#if SIZE_MAX > UINT_MAX
+      || digest_len > UINT_MAX
+#endif
+  ) {
     return 0;
   }
 
@@ -458,7 +463,7 @@ int DH_compute_key_hashed(DH *dh, uint8_t *out, size_t *out_len,
   int ret = 0;
   const size_t dh_len = DH_size(dh);
   uint8_t *shared_bytes = OPENSSL_malloc(dh_len);
-  unsigned out_len_unsigned;
+  unsigned out_len_unsigned = (unsigned)digest_len;
   if (!shared_bytes ||
       // SP 800-56A is ambiguous about whether the output should be padded prior
       // to revision three. But revision three, section C.1, awkwardly specifies